Allow filtering by trigger bits, add accessor #1670
Conversation
|
Hi @michaelmackenzie,
which require these tests: build. @Mu2e/fnalbuild-users, @Mu2e/write have access to CI actions on main. ⌛ The following tests have been triggered for 1c80479: build (Build queue - API unavailable) |
|
☀️ The build tests passed at 1c80479.
N.B. These results were obtained from a build of this Pull Request at 1c80479 after being merged into the base branch at e391635. For more information, please check the job page here. |
| Comment("Trigger line names to test; if any of these are set the event will pass the filter"), vector<string>()}; | ||
| fhicl::Sequence<unsigned> triggerBits{ Name("TriggerBits"), | ||
| Comment("Trigger line bits to test; if any of these are set the event will pass the filter"), vector<unsigned>()}; | ||
| }; |
There was a problem hiding this comment.
In the future, it would be nice if you could align the different fields, this makes the code easier to read
There was a problem hiding this comment.
I tried to better align these
| throw cet::exception("TRIGGER") << "TriggerResultsNavigator: Bit " << bit << " not found"; | ||
| } | ||
|
|
||
| // Has ith path accepted the event? |
There was a problem hiding this comment.
Please avoid implementing functions in the header because any change can trigger massive recompilation (exception: simple accessor /setter). Could you implement this in the cc file?
There was a problem hiding this comment.
I moved these to the cc file
| return accepted(name); | ||
| } | ||
|
|
||
| bool wasrun(std::string const& name) const; |
There was a problem hiding this comment.
I noticed that the trigger navigators relies a lot on strings, which is slow. IS there a way to improve this?
There was a problem hiding this comment.
I added unordered maps so we can avoid more string comparisons. Users may ask about the bit, the string name, or the index, so all of these interfaces need to be maintained. I think this version should be clearer though.
|
I updated the interface to be clearer, but this leads to downstream updates. Before it was ambiguous if an access was using/returning the bit or list index, now methods include this information in the function name. |
|
@FNALbuild run build test |
|
⌛ The following tests have been triggered for 2709cea: build (Build queue - API unavailable) |
|
☀️ The build tests passed at 2709cea.
N.B. These results were obtained from a build of this Pull Request at 2709cea after being merged into the base branch at 5d5efb9. For more information, please check the job page here. |
4 participants
No description provided.