-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
[PRO] adding new PPS conditions for 2023 #46331
Conversation
cms-bot internal usage |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-46331/42164
|
A new Pull Request was created by @diemort for master. It involves the following packages:
@antoniovagnerini, @antoniovilela, @atpathak, @civanch, @cmsbuild, @consuegs, @davidlange6, @fabiocos, @kpedro88, @mandrenguyen, @mdhildreth, @nothingface0, @perrotta, @rappoccio, @rvenditti, @syuvivida, @tjavaid can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
please test |
please test |
Is there a plan to get these data into the CondDB? (just curious) |
please test |
@makortel indeed, we had this discussion in cms-data. Therre are a few conditions that need to be moved to DB, however the old version of direct simulation is based on local conditions especially Run2. We're working on a validation procedure and can investigate about moving all conditions to DB. |
the fails seem to be in the wf |
please test there was bug in cms-bot which is fix now |
-1 Failed Tests: UnitTests RelVals Unit TestsI found 2 errors in the following unit tests: ---> test test_MC_23_crosscheck had ERRORS ---> test test_MC_23_setup had ERRORS RelVals
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-46331/42177
|
Pull request #46331 was updated. @antoniovagnerini, @antoniovilela, @atpathak, @civanch, @cmsbuild, @consuegs, @davidlange6, @fabiocos, @kpedro88, @mandrenguyen, @mdhildreth, @nothingface0, @perrotta, @rappoccio, @rvenditti, @syuvivida, @tjavaid can you please check and sign again. |
Regarding the error in the RelVal above, it may be related to the missing era in case the fix in the wf was related to a modification in Era_Run3_2023. |
Adding a comment on this point: having these conditions in the DB is not straightforward as it sounds. Since our detector and reconstruction changes conditions very often, we came up some years ago with the implementation of the so-called profiles, which are sets of conditions (not supplied from the DB) between which the direct simulation switches when changing lumisections. |
After discussion, PPS offline sw decided to stick with a single Era modifier (already in use) to update conditions from config file. I'm closing this PR and a new one will be prepared to avoid confusion. @perrotta No need to implement change/addition in Eras. |
This PR is meant to introduce the latest PPS conditions for 2023 for the PPS fast simulation (Direct Simulation).
PPS direct simulation with 2023 conditions show the expected agreement in proton reconstruction based on the gen v recpo comparison.
Not a back-port; a new PR is going to be submitted meant for 14.1 since that was the release for 2023 data-taking.
PR passed unittests and runTheMatrix 44 43 41 36 19 1 1 1 1 1 1 tests passed, 0 0 0 0 0 0 0 0 0 0 0 failed
FYI @fabferro @AndreaBellora @forthommel