forked from cms-sw/cmssw
-
Notifications
You must be signed in to change notification settings - Fork 1
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
Follow up PRs after merging cms-sw/cmssw#45117 #75
Comments
Not explicitly from the review comments:
|
VourMa
changed the title
Comments to be dealt with in follow up PRs after PR#45117
List of unresolved comments (proposal to dealt with them in follow up PRs after #45117)
Aug 19, 2024
VourMa
changed the title
List of unresolved comments (proposal to dealt with them in follow up PRs after #45117)
List of unresolved comments (proposal to deal with them in follow up PRs after #45117)
Aug 19, 2024
VourMa
changed the title
List of unresolved comments (proposal to deal with them in follow up PRs after #45117)
Follow up PRs after merging cms-sw/cmssw#45117
Nov 1, 2024
Comments provided after batch9:
|
there are other dependencies on the geometry, which are hardcoded directly or longer range, mostly re OT tracker:
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Priority: Soon after merging
Integrate accumulated algorithm developments held back during integration PR review
Update LST README, printouts and minor fixes.
Links to specific comments
Covered partially by #55. A follow up to accommodate the developments made in #72 will be needed.
Deal with the configurability of the pT threshold
Covered by #39.
LST in HLT
Changes in LST conditions data
I would prefer if the possibility of overriding the conditions data setting
LST_BASE
orTRACKLOOPERDIR
would be disabled when building this code inside CMSSW.LSTESData::geometryDataDir
needs a check thatgetenv("CMSSW_SEARCH_PATH")
is not null.LST workflow
Respect new relval_2026.py structure.
Decide if we want different workflows for different backends.
Implement a "GPU vs CPU" workflow.
Fill seed stop info when LST runs (expected by DQM/validation).
Make general-purpose functions widely available in CMSSW
The plan would be to move all general-purpose functions (work started in #71):
Priority: Short-term plan
Rewrite the loops in the kernels using uniform_elements, independent_groups, independent_group_elements, etc.
CMSSW Integration of LST cms-sw/cmssw#45117 (comment):
Experiment with
make_workdiv
.CMSSW Integration of LST cms-sw/cmssw#45117 (comment):
No need to set the grid size to 1, the alpaka execution will run the blocks one after the other automatically. You can set the grid size to 1 - the difference will be in how the kernel are executed over the elements. Either way, I think whatever approach you choose should be documented.
CMSSW Integration of LST cms-sw/cmssw#45117 (review)
Links to specific comments
Rewrite kernels with proper, concrete dimensions instead of templated types
CMSSW Integration of LST cms-sw/cmssw#45117 (review):
Links to specific comments
- [ ] https://github.com/CMSSW Integration of LST cms-sw/cmssw#45117#discussion_r1747146697 - [ ] https://github.com/CMSSW Integration of LST cms-sw/cmssw#45117#discussion_r1747161172 - [ ] https://github.com/CMSSW Integration of LST cms-sw/cmssw#45117#discussion_r1747493770 - [ ] https://github.com/CMSSW Integration of LST cms-sw/cmssw#45117#discussion_r1747495326 - [ ] https://github.com/CMSSW Integration of LST cms-sw/cmssw#45117#discussion_r1747501517 - [ ] https://github.com/CMSSW Integration of LST cms-sw/cmssw#45117#discussion_r1747519297 - [ ] https://github.com/CMSSW Integration of LST cms-sw/cmssw#45117#discussion_r1747520469 - [ ] https://github.com/CMSSW Integration of LST cms-sw/cmssw#45117#discussion_r1748005531 - [ ] https://github.com/CMSSW Integration of LST cms-sw/cmssw#45117#discussion_r1748005961 - [ ] https://github.com/CMSSW Integration of LST cms-sw/cmssw#45117#discussion_r1748006364 - [ ] https://github.com/CMSSW Integration of LST cms-sw/cmssw#45117#discussion_r1748008415 - [ ] https://github.com/CMSSW Integration of LST cms-sw/cmssw#45117#discussion_r1748008961 - [ ] https://github.com/CMSSW Integration of LST cms-sw/cmssw#45117#discussion_r1748009215 - [ ] https://github.com/CMSSW Integration of LST cms-sw/cmssw#45117#discussion_r1748009259 - [ ] https://github.com/CMSSW Integration of LST cms-sw/cmssw#45117#discussion_r1748009395 - [ ] https://github.com/CMSSW Integration of LST cms-sw/cmssw#45117#discussion_r1748009581 - [ ] https://github.com/CMSSW Integration of LST cms-sw/cmssw#45117#discussion_r1748009869 - [ ] https://github.com/CMSSW Integration of LST cms-sw/cmssw#45117#discussion_r1748010016 - [ ] https://github.com/CMSSW Integration of LST cms-sw/cmssw#45117#discussion_r1748010483 - [ ] https://github.com/CMSSW Integration of LST cms-sw/cmssw#45117#discussion_r1748010582 - [ ] https://github.com/CMSSW Integration of LST cms-sw/cmssw#45117#discussion_r1748010772 - [ ] https://github.com/CMSSW Integration of LST cms-sw/cmssw#45117#discussion_r1748013396 - [ ] https://github.com/CMSSW Integration of LST cms-sw/cmssw#45117#discussion_r1748058899 - [ ] https://github.com/CMSSW Integration of LST cms-sw/cmssw#45117#discussion_r1749143631 - [ ] https://github.com/CMSSW Integration of LST cms-sw/cmssw#45117#discussion_r1749143810 - [ ] https://github.com/CMSSW Integration of LST cms-sw/cmssw#45117#discussion_r1749143984 - [ ] https://github.com/CMSSW Integration of LST cms-sw/cmssw#45117#discussion_r1749144167 - [ ] https://github.com/CMSSW Integration of LST cms-sw/cmssw#45117#discussion_r1749163156 - [ ] https://github.com/CMSSW Integration of LST cms-sw/cmssw#45117#discussion_r1749163618 - [ ] https://github.com/CMSSW Integration of LST cms-sw/cmssw#45117#discussion_r1749163821 - [ ] https://github.com/CMSSW Integration of LST cms-sw/cmssw#45117#discussion_r1749164831 - [ ] https://github.com/CMSSW Integration of LST cms-sw/cmssw#45117#discussion_r1749165439 - [ ] https://github.com/CMSSW Integration of LST cms-sw/cmssw#45117#discussion_r1749236366 - [ ] https://github.com/CMSSW Integration of LST cms-sw/cmssw#45117#discussion_r1749237268 - [ ] https://github.com/CMSSW Integration of LST cms-sw/cmssw#45117#discussion_r1749237704 - [ ] https://github.com/CMSSW Integration of LST cms-sw/cmssw#45117#discussion_r1749237801 - [ ] https://github.com/CMSSW Integration of LST cms-sw/cmssw#45117#discussion_r1749237960 - [ ] https://github.com/CMSSW Integration of LST cms-sw/cmssw#45117#discussion_r1749238020 - [ ] https://github.com/CMSSW Integration of LST cms-sw/cmssw#45117#discussion_r1749238089 - [ ] https://github.com/CMSSW Integration of LST cms-sw/cmssw#45117#discussion_r1749238179 - [ ] https://github.com/CMSSW Integration of LST cms-sw/cmssw#45117#discussion_r1749238234 - [ ] https://github.com/CMSSW Integration of LST cms-sw/cmssw#45117#discussion_r1749238303 - [ ] https://github.com/CMSSW Integration of LST cms-sw/cmssw#45117#discussion_r1749238386 - [ ] https://github.com/CMSSW Integration of LST cms-sw/cmssw#45117#discussion_r1749238443 - [ ] https://github.com/CMSSW Integration of LST cms-sw/cmssw#45117#discussion_r1749238487 - [ ] https://github.com/CMSSW Integration of LST cms-sw/cmssw#45117#discussion_r1833303542Priority: Long-term plan
Improvements to the data format interface between LST and CMSSW
CMSSW Integration of LST cms-sw/cmssw#45117 (review):
Define the LST inputs and output as PortableCollections, and pass them (or their Views) to the LST algorithm.
Links to specific comments
Fill a host SoA directly in
LSTPhase2OTHitsInputProducer
, copy it to device, and avoid the 6 intermediate copies of thestd::vector
s.Performance-wise best would be to move them in
LSTProducer::produce()
, that would require theLST::hits()
etc to either return a mutable reference, or a value that was moved-from in thehits()
method itself.Eventually (or ideally) we'd want to issue the
alpaka::memcpy()
calls to copy the data from device to host inacquire()
, and have the destination host memory accessed only inproduce()
.Construct LST condition data from the existing EventSetup info
Improvements in storing and using "magic numbers
CMSSW Integration of LST cms-sw/cmssw#45117 (review):
Links to specific comments
Miscellaneous improvements
Try std::binary_search(data, data + ndata, search_val) from C++20.
Review uint4 usage.
Improvements to ESProducer
Make it more clear which members are on the device memory and which are on the host memory. Suggest to consider moving all data that are used only in host code into a separate ES data product.
The text was updated successfully, but these errors were encountered: