Added macro for generating element-wise methods #47625
Conversation
|
cms-bot internal usage |
|
A new Pull Request was created by @leobeltra for master. It involves the following packages:
@cmsbuild, @fwyzard, @makortel can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
enable gpu |
|
please test |
|
-1 Failed Tests: rocmUnitTests ROCm Unit TestsI found 1 errors in the following unit tests: ---> test testRocmSoALayoutAndView_t had ERRORS Comparison SummarySummary:
CUDA Comparison SummarySummary:
ROCM Comparison SummaryThere are some workflows for which there are errors in the baseline: Summary:
|
|
The ROCm failure is expected, as it is testing some error conditions... but I guess we need to rewrite the test so that it report it as "success" rather than "failure". |
|
ignore tests-rejected with ib-failure |
| BOOST_PP_EMPTY()) | ||
|
|
||
| /* Preprocessing loop for managing functions generation: only macros containing valid content are expanded */ | ||
| #define ENUM_FOR_PRED(r, state) BOOST_PP_LESS(BOOST_PP_TUPLE_ELEM(0, state), BOOST_PP_TUPLE_ELEM(1, state)) |
There was a problem hiding this comment.
Sorry, I missed this earlier.
Here and below, can you change the arguments to uppercase, to keep the same convention as the rest of the code ?
| #define ENUM_FOR_PRED(r, state) BOOST_PP_LESS(BOOST_PP_TUPLE_ELEM(0, state), BOOST_PP_TUPLE_ELEM(1, state)) | |
| #define ENUM_FOR_PRED(R, STATE) BOOST_PP_LESS(BOOST_PP_TUPLE_ELEM(0, STATE), BOOST_PP_TUPLE_ELEM(1, STATE)) |
Also, what are 0 and 1 here ?
There was a problem hiding this comment.
Those 0and 1 are the arguments of state, a tuple composed by: a number starting from 0 and incrementing at every step, until the second argument, that is the size of the variadic sequence. The third argument is the sequence which will be unrolled. This BOOST_PP_FOR loop is needed to let the user define methods with an arbitrary number of commas inside the macro.
|
@leobeltra do you have a preference if we should merge first #47306 or #47625 (this PR) ? |
We could merge #47306 first since it will be easier to rebase this on top of that. |
| @@ -2,4 +2,4 @@ | |||
| <use name="DataFormats/Portable"/> | |||
| <use name="DataFormats/SoATemplate"/> | |||
| <use name="catch2"/> | |||
| </bin> | |||
| </bin> | |||
There was a problem hiding this comment.
Please add back the final newline.
There was a problem hiding this comment.
In fact, this file and package should not have any changes.
DataFormats/SoATemplate/README.md
Outdated
| @@ -52,6 +52,10 @@ of elements in the SoA), `byteSize()`, `byteAlignment()`, `data()` (a pointer to | |||
| function computes the first byte of a structure right after a layout, allowing using a single buffer for multiple | |||
| layouts. | |||
|
|
|||
| ## Customized methods | |||
|
|
|||
| It is possible to generate methods inside the `element` and `const_element` nested structs using the `SOA_METHODS` and `SOA_CONST_METHODS` macros. More than one declaration of these macros are not allowed and all the customized methods can be implemented as macro argument. [An example is showed below.](#examples) | |||
There was a problem hiding this comment.
| It is possible to generate methods inside the `element` and `const_element` nested structs using the `SOA_METHODS` and `SOA_CONST_METHODS` macros. More than one declaration of these macros are not allowed and all the customized methods can be implemented as macro argument. [An example is showed below.](#examples) | |
| It is possible to generate methods inside the `element` and `const_element` nested structs using the `SOA_METHODS` and `SOA_CONST_METHODS` macros. Each of these macros can be called only once, and can define multiple methods. [An example is showed below.](#examples) |
| y() *= arg; | ||
| z() *= arg; | ||
| } | ||
| ), |
| SOA_ELEMENT_METHODS(void normalise() { | ||
| float norm_position = square_norm_position(); | ||
| if (norm_position > 0.0f) { | ||
| x() /= norm_position; | ||
| y() /= norm_position; | ||
| z() /= norm_position; | ||
| }; | ||
| double norm_velocity = square_norm_velocity(); | ||
| if (norm_velocity > 0.0f) { | ||
| v_x() /= norm_velocity; | ||
| v_y() /= norm_velocity; | ||
| v_z() /= norm_velocity; | ||
| }; | ||
| }), |
There was a problem hiding this comment.
| SOA_ELEMENT_METHODS(void normalise() { | |
| float norm_position = square_norm_position(); | |
| if (norm_position > 0.0f) { | |
| x() /= norm_position; | |
| y() /= norm_position; | |
| z() /= norm_position; | |
| }; | |
| double norm_velocity = square_norm_velocity(); | |
| if (norm_velocity > 0.0f) { | |
| v_x() /= norm_velocity; | |
| v_y() /= norm_velocity; | |
| v_z() /= norm_velocity; | |
| }; | |
| }), | |
| SOA_ELEMENT_METHODS( | |
| void normalise() { | |
| float norm_position = square_norm_position(); | |
| if (norm_position > 0.0f) { | |
| x() /= norm_position; | |
| y() /= norm_position; | |
| z() /= norm_position; | |
| }; | |
| double norm_velocity = square_norm_velocity(); | |
| if (norm_velocity > 0.0f) { | |
| v_x() /= norm_velocity; | |
| v_y() /= norm_velocity; | |
| v_z() /= norm_velocity; | |
| }; | |
| } | |
| ), |
that is, move the method to a new line, like in the example below.
| const { return sqrt(v_x() * v_x() + v_y() * v_y() + v_z() * v_z()); }; | ||
|
|
||
| template <typename T1, typename T2> | ||
| auto time(T1 pos, T2 vel) const { |
There was a problem hiding this comment.
This function does not use any of the element data members.
Should it be marked as static ?
There was a problem hiding this comment.
The examples mix the syntax view[i].x() and view.x()[i] in a seemingly random way.
Is it intended to show that both syntaxes work ?
Otherwise I would prefer to consistently use the former, view[i].x().
|
I trust you on the macro definitions :-) |
|
I have only trivial comments about the documentation and test. |
|
Now that #47306 has been merged, this PR needs to be rebased. |
cmsbuild
added
tests-pending
code-checks-pending
and removed
tests-approved
code-checks-approved
tests-ib-failure
labels
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-47625/44394
|
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-47625/44430
|
PR description:
Added two new macros:
SOA_ELEMENT_METHODSandSOA_CONST_ELEMENT_METHODS. The user can declare functions with an arbitrary number of input and template arguments inside these macros. The functions will act on the elements (rows) of the SoA. A static assert has been added to ensure no multiple calls to these macros within the main macro are done.PR validation:
Added a unit test that passes.