-
-
Notifications
You must be signed in to change notification settings - Fork 187
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add unary operations minus and logical negation to kernel generator #1824
Add unary operations minus and logical negation to kernel generator #1824
Conversation
…4.1 (tags/RELEASE_600/final)
…stable/2017-11-14)
…4.1 (tags/RELEASE_600/final)
@serban-nicusor-toptal This PR has been reformated 3 times in a row. It seems there are still some differences in clang format, even if they are all nominally 6.0.0. |
Hey, would it help if I spin up a machine and send you credentials maybe you can debug it ? |
The difference is probably in the exact version: See Can you check which one was the second one and see if we can put it on the same tag? |
@rok-cesnovar |
I am not an expert on this, but my hunch is that ye, aynthing with |
Okay thanks, on it. |
Hmmh the job passed clang-format now.
So I think what's causing issues is actually the legacy clang-format on
Going to make a backup of the legacy clang-format on This will take a little bit of time, I need to get sudo access on the machine. Sorry! Edit: Done! |
Jenkins Console Log Machine informationProductName: Mac OS X ProductVersion: 10.11.6 BuildVersion: 15G22010CPU: G++: Clang: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm! Once quick Q though, should this PR also include the macro for defining other unary ops?
typename std::remove_reference_t<T>::Scalar> { | ||
using base = unary_operation_cl<unary_minus_<T>, T, | ||
typename std::remove_reference_t<T>::Scalar>; | ||
using base::arguments_; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this get used somewhere else?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was planning to do a refactor that would make derived classes use operation_cl::arguments_
directely again instead of get_argument
(possibly before this PR was merged). At the end I did not do it in this PR, but it is comming.
About macro: this PR already adds both that make sense (I don't plan to support in place ones like ++). Custom functions can be added with the macros in unary_function_cl.hpp. |
Summary
Adds unary operations (unary minus and logical negation) to kernel generator.
Tests
New operations are tested.
Side Effects
None.
Release notes
Added unary operations (unary minus and logical negation) to kernel generator.
Checklist
Math issue Implement OpenCL kernel generator #1342
Copyright holder: Tadej Ciglarič
The copyright holder is typically you or your assignee, such as a university or company. By submitting this pull request, the copyright holder is agreeing to the license the submitted work under the following licenses:
- Code: BSD 3-clause (https://opensource.org/licenses/BSD-3-Clause)
- Documentation: CC-BY 4.0 (https://creativecommons.org/licenses/by/4.0/)
the basic tests are passing
./runTests.py test/unit
)make test-headers
)make test-math-dependencies
)make doxygen
)make cpplint
)the code is written in idiomatic C++ and changes are documented in the doxygen
the new changes are tested