-
Notifications
You must be signed in to change notification settings - Fork 178
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
refactor: Examples sequence elements gain Read/WriteDataHandles #1908
refactor: Examples sequence elements gain Read/WriteDataHandles #1908
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1908 +/- ##
=======================================
Coverage 49.55% 49.55%
=======================================
Files 408 408
Lines 22695 22695
Branches 10356 10356
=======================================
Hits 11247 11247
Misses 4239 4239
Partials 7209 7209 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
9dc0b33
to
0214b8b
Compare
This should be ready for review now, @benjaminhuth @andiwand |
6369276
to
71b6e44
Compare
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.
gave it a quick look. makes a lot of sense to me!
Examples/Framework/include/ActsExamples/Framework/IAlgorithm.hpp
Outdated
Show resolved
Hide resolved
Examples/Framework/include/ActsExamples/Framework/WhiteBoard.hpp
Outdated
Show resolved
Hide resolved
Examples/Io/Root/include/ActsExamples/Io/Root/RootVertexPerformanceWriter.hpp
Show resolved
Hide resolved
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.
nice! looks good to me 👍
📊 Physics performance monitoring for 4f731e9Full report VertexingSeedingCKFAmbiguity resolutionTruth tracking (Kalman Filter)Truth tracking (GSF) |
Ok I think what this failure is telling is, that I can't make only the ODD sequence use the handles and have the Sequencer fail. So for this PR I'll downgrade the Sequencer ERROR to a warning, and won't throw. The data flow will still be correct, and the CI should tell us. I have a follow-up PR (almost) ready where I switch everything over and can reenable the ERROR and exception. |
Ok, I've added a config flag to the sequencer that defaults to False for the checking functionality. I'll default it to true with the other PR I was mentioning. Thoughts? |
sounds good to me 👍 |
Follow up to #1908. Switches over the rest of the Examples components. This also makes `WhiteBoard::{get,add}` private and adds the data handles as friends to restrict unregistered white board access.
This locks in the type at algorithm construction, registers the handles and allows checking the data flow more thoroughly before running the event sequence.
This PR only changes the components used in the ODD chain example, but we should probably extend this to the rest of the Examples components as well.
Blocked by: