-
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
Add new dEdx calibration and estimator #45016
Conversation
cms-bot internal usage |
"process.GlobalTag.toGet.extend([cms.PSet(record = cms.string("DeDxCalibrationRcd"), tag = cms.string("dedxCalibration"), connect = cms.string("sqlite_file:../dedxCalibration.db"))])"' shouldn't we put this condition in the GT? |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-45016/40302
|
A new Pull Request was created by @stahlleiton for master. It involves the following packages:
@cmsbuild, @francescobrivio, @saumyaphor4252, @perrotta, @mandrenguyen, @jfernan2, @consuegs can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
Yes, but I thought in order to add to the GT the condition format had to exist in CMSSW. Is it possible to add to the database before adding the cond format in CMSSW? |
it is possible, see e.g.: https://cms-conddb.cern.ch/cmsDbBrowser/list/Prep/tags/DeDxCalibration_test_PR_45016 that I did for exercise. |
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.
some unrequested comments by a non-reviewer (this PR caught my eye as a former domain maintainer of some of these packages).
ok, then maybe a better strategy would be to separate this PR into two, one with the cond format changes and another with the dEdx code changes, and get the cond format changes merged first. |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-45016/40309
|
Pull request #45016 was updated. @jfernan2, @saumyaphor4252, @consuegs, @mandrenguyen, @cmsbuild, @francescobrivio, @perrotta can you please check and sign again. |
That is indeed what's (generally) done in such cases. If this is urgent it can conceivably still enter in one go (upon agreement with alca/db, since it requires some manual hack). |
Seems the tests worked at the end |
As far as I understand this is expected to work by construction, because the input files are reused from an existing Relval test
therefore they have already been cached in the |
+1 |
+pdmv |
@cms-sw/upgrade-l2 : please review/sign this PR , thanks |
+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. @rappoccio, @sextonkennedy, @antoniovilela (and backports should be raised in the release meeting by the corresponding L2) |
+1 |
starting from
All of them have, see e.g. this log:
this PR is likely the culprit. |
Thanks for reporting it. I will work on addressing this issue asap, but I wonder why this was not found by the PR tests. |
The failing workflows are not run as part of PR tests. |
ok, thanks for the clarification. Apart from 136.901, 136.902, 136.903, 136.904, are there any other relvals that are also failing (to test against all that are failing) |
In the last IBs there are many other relval failures (https://cmssdt.cern.ch/SDT/html/cmssdt-ib/#/ib/CMSSW_14_1_X), but none of them seems to be related to this PR. |
The PR #45288 address this issue |
PR description:
This PR adds a new dEdx estimator meant for the 2024 PbPb UPC reco. It includes the following changes on top of "Run3_UPC" era:
More details on the changes and validations done are documented in the slides
The code was adapted from Ferenc Sikler's dEdx framework and results. The details are also documented in the CADI HIN-12-016.
This PR requires #45024
@mandrenguyen @sikler
PR validation:
Tested workflows 180, 180.1, 181 and 181.1 with the dedxCalibration.db file using:
runTheMatrix.py -l 180,180.1,181,181.1 --command '--customise_commands "process.GlobalTag.toGet.extend([cms.PSet(record = cms.string("DeDxCalibrationRcd"), tag = cms.string("dedxCalibration"), connect = cms.string("sqlite_file:../dedxCalibration.db"))])"'
If this PR is a backport please specify the original PR and why you need to backport that PR. If this PR will be backported please specify to which release cycle the backport is meant for: