-
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
Use --beamspot Nominal2022PbPbCollision
for all Run 3 heavy-ion workflows
#40653
Use --beamspot Nominal2022PbPbCollision
for all Run 3 heavy-ion workflows
#40653
Conversation
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-40653/33975
|
A new Pull Request was created by @mandrenguyen (Matthew Nguyen) for master. It involves the following packages:
@bbilin, @cmsbuild, @AdrianoDee, @srimanob, @kskovpen, @sunilUIET can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
test parameters:
|
@cmsbuild please test |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-08a26e/30323/summary.html Comparison SummarySummary:
|
Polite ping to review/sign for @cms-sw/upgrade-l2 @cms-sw/pdmv-l2 |
+pdmv |
+Upgrade |
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 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) |
please test |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-08a26e/30789/summary.html Comparison SummarySummary:
|
@mandrenguyen changes in the 159.3 workflow are rather important, with overall a larger number of objects reconstructed of all kinds (calo, tracks, muons...). I imagine they can all be compatible with the updated beam spot, but could you please confirm that they are all expected? |
this certainly looks more correct than before |
@perrotta Marco is probably better positioned than I am to immediately know how to diagnose whether the vertex smearing and beamspot are compatible. To reiterate, when we moved to the new vertex smearing we actually broke these workflows, because they were not using the corresponding GT. This new choice of GT has a beamspot tag that was derived using precisely this vertex smearing. If there's a doubt, I think @cms-sw/alca-l2 can comment or sign. |
+1 |
Just for the record,
It looks like the centroid is actually in the same position in the two vertex smearings and both are compatible with the beamspot used in global tag. On the other hand the old vertex smearing clearly disagrees with the payload in Global Tag in terms of longitudinal width (and probably also in terms of transverse beam width, but I didn't do the math using the emittance / beta*). |
PR description:
Back in #39010 I updated the Run 3 heavy ion relval workflows to use their own dedicated vertex smearing, instead of the one inherited from pp. This was only done for the workflow running Hydjet (actual heavy-ion events). I forgot to also update the "noPU" workflows that just run pp collisions, but with the heavy-ion era. Since those wfs use the same beamspot tag, they were running with an inconsistent vertex smearing and reco beamspot. This PR fixes that. I also did a bit of cleanup.
PR validation:
I only tested that
runTheMatrix.py -n -e
doesn't crash.We should trigger 159.02, 159.3 and 310
I don't think we need a backport.
@prebello