-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Update MkFit to support multiple iterations #33802
Conversation
-code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33802/22788 ERROR: Build errors found during clang-tidy run.
|
@smuzaffar @mrodozov How can I ask code-checks to be run with a cmsdist PR? I did not understand how to apply the command suggested in the PR command list
|
@makortel , one should first start the test for cmsdist PR. That will build externals and deploy them on cvmfs. On the summary page of cmsdist PR, there should be a link for cmssw-tool-conf which should be used starting the code-ckechs. I have updated the http://cms-sw.github.io/cms-bot-cmssw-cmds.html to include this information |
code-checks with cms.week1.PR_a23f90cc/46.0-f33a7e654ce2bd583f9cc1a2001b31c1 |
-code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33802/22791
Code check has found code style and quality issues which could be resolved by applying following patch(s)
|
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.
The JSON files for per-iteration parameters will be moved to cms-data (and removed from the commit history) after the PR review is otherwise complete.
how often are these files supposed to change, if at all?
Thanks @smuzaffar! |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33802/22800
|
Years of experience have amply demonstrated that very little is "temporary"
in the CMS software.
|
+Upgrade The mkfit workflow, 11634.7, is dumped properly and run until the end. Note that,
|
Does @cms-sw/hlt-l2 prefer to not have the customize function then? |
@makortel I've asked Trigger Coordination, and the answer has been that TSG does not plan to use it or support this customisation; and it must not prevent any future HLT updated from being promptly merged in CMSSW, even if it breaks this customisation. So, the customisation can go in, but only if O&C agrees to never consider it a blocking issue for merging HLT updates. |
The X.7 workflow is not part of the tests run in PR tests, so a HLT update causing the X.7 workflow to break would not be caught in the PR tests but only in IBs (and therefore would not block HLT update PRs). If/when X.7 workflow breaks, I'd be happy to fix it (or remove the customization if a quick fix would be impractical) without any effort from HLT. @qliphy @silviodonato @smuzaffar Would you find this acceptable? |
+1 I'm also curious about the plan for validation. |
Silvio and me are fine with this. @cms-sw/hlt-l2 Do you have further comments? |
Thanks @qliphy @silviodonato . I think TSG is OK with tihs approach. |
+hlt |
+1 |
This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will be automatically merged. |
PR description:
This PR updates MkFit to support multiple tracking iterations. For now, preliminary tuning, and an option to switch pattern recognition to MkFit, is provided for initialStep, lowPtQuadStep, highPtTripletStep, lowPtTripletStep, detachedQuadStep, pixelLessStep, and tobTecStep. Work for tuning for these as well as the later iterations continues in parallel. This PR adds Modifiers for each of these iterations (plus detachedTripletStep and mixedTripletStep) to allow switching any set of them to be switched to MkFit, and a ModifierChain to switch them all (this is intended only as a temporary solution, with a single Modifier to switch all iterations seen as the longer term solution once a satisfactory tuning has been achieved).
In addition this PR includes
tbb::task_arena
, in order to be able to account all computations in EDModule timing measurements (like framework timers or FastTimerService)In addition to the physics tuning there are several technical issues remaining that will be addressed in future PR(s).
The JSON files for per-iteration parameters will be moved to cms-data (and removed from the commit history) after the PR review is otherwise complete.
Requires cms-sw/cmsdist#6917
FYI @mtosi @vmariani @mmusich
PR validation:
Tested workflows 11634.0 and 11634.7 run.
MTV plots comparing MkFit (enabled in all the aforementioned iterations) to CKF can be found from http://uaf-10.t2.ucsd.edu/~mmasciov/MkFit_multiIteration_forPR/plots_500evts_lessIters_all/. The plots were done in CMSSW_11_2_0 with ttbar+PU 50 with realistic 2021 conditions. The mkFit was compiled with gcc for SSE3 in red (setup in the external pull request), and icc for AVX2 in black, run on Skylake Gold on a single thread.