-
Notifications
You must be signed in to change notification settings - Fork 34
Update OverlayTiming to support BIB random mixing #413
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
Open
madbaron
wants to merge
19
commits into
key4hep:main
Choose a base branch
from
madbaron:add_overlay_BIB_random_mix
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 18 commits
Commits
Show all changes
19 commits
Select commit
Hold shift + click to select a range
5c76efa
merging randomMix functionalities into overlaytiming
madbaron 54afad5
docs
madbaron 4c17050
more docs
madbaron 9ef1063
undo over-eager release notes
madbaron fdfde27
phrasing of pseudo-events
madbaron a315f27
use TBB
madbaron 24b2107
clang format
madbaron b517240
update MT docs
madbaron 2fb0c4f
Merge branch 'main' into add_overlay_BIB_random_mix
madbaron d3758ff
Merge branch 'main' into add_overlay_BIB_random_mix
madbaron a983a90
implement suggestion from ArinaPon
madbaron 9aab53b
Fix clang-format in OverlayTiming.cpp
madbaron 349e589
updated comments and added shuffle on wrap
madbaron b8b4f57
make pre-commit happy again
madbaron 0fad14e
merge with upstream
madbaron 33f9829
Merge branch 'add_overlay_BIB_random_mix' of github.com:madbaron/k4FW…
madbaron 6e6b20c
preempt clang tidy
madbaron 3b80ce7
fix clang-tidy
madbaron a25b300
Merge branch 'main' into add_overlay_BIB_random_mix
madbaron File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Conceptually this might interfere with Gaudis internal scheduling (even if we also use tbb to do our multithreading). It's unclear to me whether the Gaudi internal tbb bits communicate with the tbb bits here.
There is precedent for doing this though as the CKF in k4ActsTracking also does some internal multithreading. This might need some policy discussion as it could imply different usage patterns for different community (e.g. run the general chain on a single thread but branch out to multi-threading in dedicated algorithms vs. running the full chain on multiple threads with Gaudi scheduling but no algorithm-internal multi-threading).
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.
Indeed. In the ideal world you might want to allow users to do a combination of both, if possible.
For now, especially in colliders that are computationally challenging per event, being able to use MT inside the same event is much more important than multi-threading over events, which can be done trivially in batch jobs anyway.
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.
Maybe @jmcarcell knows if functional algorithms can already propagate that to the Gaudi scheduler somehow. Otherwise the potential interplay will for now just be another thing to document.