-
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 SimBeamSpotObjects to handle different EvtVtxGenerators #43160
Update SimBeamSpotObjects to handle different EvtVtxGenerators #43160
Conversation
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-43160/37467
|
A new Pull Request was created by @francescobrivio for master. It involves the following packages:
@perrotta, @saumyaphor4252, @mdhildreth, @francescobrivio, @civanch, @consuegs can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-7e0423/35538/summary.html Comparison SummarySummary:
|
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.
Thanks @francescobrivio, I have a small suggestion (open to discussion)
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-43160/37475
|
Pull request #43160 was updated. @consuegs, @cmsbuild, @mdhildreth, @civanch, @saumyaphor4252, @francescobrivio, @perrotta can you please check and sign again. |
a773608
to
c7f6425
Compare
Pull request #43160 was updated. @mdhildreth, @francescobrivio, @cmsbuild, @saumyaphor4252, @civanch, @consuegs, @perrotta can you please check and sign again. |
please test |
in the interest of starting to have something usable for Run-3 simulation, I would defer this step to a later PR. |
Ok as you prefer! |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-7e0423/35566/summary.html Comparison SummarySummary:
|
+db |
+alca |
+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 now be reviewed by the release team before it's merged. @antoniovilela, @rappoccio, @sextonkennedy (and backports should be raised in the release meeting by the corresponding L2) |
+1 |
PR description:
This PR aims at improving the
SimBeamSpotObjects
CondFormat in the context of cms-AlCaDB/AlCaTools#86.Following the discussion in the same issue the decision is to:
SimBeamSpotObjects
to be able to handle both Betafunc- and Gauss-EvtVtxGenerators, which are the two generators used up to Run 3 (for realistic and ideal BS conditions, respectively). This is done in commit 6d6fac2, specifically in this commit I have:SimBeamSpotObjects.h
CondFormatGaussEvtVtxGenerator.h
the possibility to read the VtxSmearing parameters from the DBSimBeamSpotObjects
single-IOV tags to the Prep CondDBThe next step is to introduce a new CondFormat (and relative utility scripts) to handle the HL-LHC BeamSpot case, this is why I'm opening this PR as draft for the moment.To do:Add new CondFormat for HL-LHC BeamSpotAdd write and read scripts with relative unitTestsAdd PayloadInspector classUpdate theHLLHCEvtVtxGenerator
For this second step it would be very helpful to receive feedback on the discussion at cms-AlCaDB/AlCaTools#86 (comment) from the original authors of the HLLHCEvtVtxGenerator module (FYI @lgray).EDIT: it has been decided to leave this second step to a following PR.
PR validation:
This PR was validated by:
Which result in the following plots:
Backport:
Not a backport, no backport is foreseen for the moment.
FYI @mmusich @dzuolo @lguzzi @gennai