Remove excess selector code #1587
Conversation
for more information, see https://pre-commit.ci
| @@ -90,23 +105,33 @@ template <auto... MemberFunctionPtrs> | |||
| class ValueSplit { | |||
| public: | |||
|
|
|||
| ValueSplit(std::vector<std::vector<int>> ids) : m_ids(ids) {}; | |||
| ValueSplit( std::vector<std::vector<int>> ids, bool matching = true) | |||
There was a problem hiding this comment.
Can you also const ref the ids arg here?
There was a problem hiding this comment.
Also, optionally, I've wondered about a TrueFalseSplit functor a number of times. That's really just a ValueSplit (which is how this gets implemented in plugins), but it could have the benefit of accepting a function that returns a bool, which is semantically cleaner than relying on bool to integer conversions.
There was a problem hiding this comment.
You would have to move away from the simple std::find implementation for anything other than ==. I'll have another think, as changing the functors already there is making them less clear. It should be possible to pass an array of boolean operators along with values and member functions to a functor.
After the vertexing discussion, access to min/max might be nice too.
There was a problem hiding this comment.
BooleanSplit (name can still be changed) should be able to perform the jobs of RangeSplit and ValueSplit where an array of functions which return a bool from two inputs is passed alongside the array of values to compare. The bools are all reduced by and rather than trying to mix in some more complexity.
What is missing the the ability to compare types other than float, I'm not sure how to approach for instance asking for e.g. ReconstructedParticles with PDG "equal_to" and a goodnessOfPID "greater_than". Maybe it's not important, could be done in a few steps, or this sort of thing is not what we'll want to do in eicrecon.
for more information, see https://pre-commit.ci
wdconinc
force-pushed
the
Remove-Excess-Selector-Code
branch
from
20cb0b4
to
9786e24
Compare
|
Rebased to pick up #1591. |
… into Remove-Excess-Selector-Code
for more information, see https://pre-commit.ci
… into Remove-Excess-Selector-Code
|
Briefly, what does this PR introduce?
Removes remaining selector algorithms and factories reducing the code base.
Functionality to simply select a subset collection based on not matching was missing from the simple sub-divide functors (selecting all charged particles). This PR extends the ValueSplit to allow the output to each collection where the filter does not match.
Working the same in the case of selecting charged particles, the option to select outside of a range in RangeSplit has been added but this is slightly less intuitive and probably less optimal (charge<0||charge>0)
Alternative approaches would be to have a dedicated separate functors for these cases or just create the lambda function in the factory definition itself.
What kind of change does this PR introduce?
Please check if this PR fulfills the following:
Does this PR introduce breaking changes? What changes might users need to make to their code?
No
Does this PR change default behavior?
No