-
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
[12_5_X] Use Era Run3_pp_on_PbPb_approxSiStripClusters in DQM clients with tracking #39975
[12_5_X] Use Era Run3_pp_on_PbPb_approxSiStripClusters in DQM clients with tracking #39975
Conversation
A new Pull Request was created by @francescobrivio for CMSSW_12_5_X. It involves the following packages:
@emanueleusai, @ahmad3213, @cmsbuild, @jfernan2, @syuvivida, @pmandrik, @micsucmed, @rvenditti can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
FYI @cms-sw/alca-l2 @cms-sw/tracking-pog-l2 @mandrenguyen |
please test |
It can not be. The Modifiers (which is what Era's use) are applied as we load python modules so one can not change what Modifiers are to be used once the Process has started to be built. |
Ok got it! thanks Chris! |
-1 Failed Tests: UnitTests RelVals-INPUT Unit TestsI found errors in the following unit tests: ---> test TestDQMOnlineClient-beam_dqm_sourceclient had ERRORS ---> test TestDQMOnlineClient-sistrip_dqm_sourceclient had ERRORS ---> test TestDQMOnlineClient-pixel_dqm_sourceclient had ERRORS RelVals-INPUTThe relvals timed out after 4 hours. Comparison SummarySummary:
|
Ok as expected this does not run on old data, but @makortel suggested privately a way around this. |
ca3bd49
to
2b94df4
Compare
Pull request #39975 was updated. @emanueleusai, @ahmad3213, @cmsbuild, @jfernan2, @syuvivida, @pmandrik, @micsucmed, @rvenditti can you please check and sign again. |
In the last push I'm using the With this last commit tests should pass and this PR can even be integrated in the release, provided @cms-sw/dqm-l2 agrees. I will also open the master PR. |
@cmsbuild please test |
I guess this doesn't apply anymore. |
I just updated the description :D |
backport of #39988 |
Pull request #39975 was updated. @emanueleusai, @ahmad3213, @cmsbuild, @jfernan2, @syuvivida, @pmandrik, @micsucmed, @rvenditti can you please check and sign again. |
@cmsbuild please test |
urgent
|
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-f0daa8/28819/summary.html Comparison SummarySummary:
|
PR added on top of 12_5_2 to online machines and tested successfully during the HI run |
+1 |
This pull request is fully signed and it will be integrated in one of the next CMSSW_12_5_X IBs (tests are also fine) and once validation in the development release cycle CMSSW_12_6_X is complete. This pull request will now be reviewed by the release team before it's merged. @perrotta, @dpiparo, @rappoccio (and backports should be raised in the release meeting by the corresponding L2) |
+1 |
PR description:
Backport of #39988
Following the addition of a new Era (
Run3_pp_on_PbPb_approxSiStripClusters
) in #39863, this PR updates the Era in the online DQM clients that use tracking in order to properly cope with theapproxSiStripClusters
.@mmusich please advise if I forgot any, or if other modifications are needed
@cms-sw/dqm-l2 I believe this is the only way to properly handle the HI run, but if you have a better plan please let me know and I can update the PR
- Note1 -
Unfortunately I don't know if acms.Process()
can be customized with a different Era (e.g. following a check on the runKey) after it has been initialized with another one (I guess not, but maybe @makortel @Dr15Jones can advise), so the hardcoded solution in this PR is to completely change the Era to theapproxSiStripClusters
one.Hence this PR should not be merged, but only applied online in DQM machines in P5 for the HI data taking period (currently scheduled for 17-18th November).
EDIT: I'm now using the
sys.argv
to determine the run key and set the proper Era when the cms.Process() is declared,so this PR should be good for both HI and pp collisions and thus can be merged.
- Note2 -
Visualization clients make use of scenarios, they will be modified in a different PR which includes a new scenario (working on it right now).
PR validation:
Tested locally with:
cmsRun DQM/Integration/python/clients/beam_dqm_sourceclient-live_cfg.py unitTest=True runkey=hi_run
with the HI input streamer files provided in this CMSTalk thread.
Backport:
Backport of #39988