-
Notifications
You must be signed in to change notification settings - Fork 4.6k
Remove EGM Scale and Smearing for Run2 reprocessing and update old EPcombination parameters #48539
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
Remove EGM Scale and Smearing for Run2 reprocessing and update old EPcombination parameters #48539
Conversation
…n the reapplication of the scale and smearing in Mini and Nano)
|
cms-bot internal usage |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-48539/45499 |
|
A new Pull Request was created by @see-saw28 for master. It involves the following packages:
@cmsbuild, @ftorrresd, @hqucms, @jfernan2, @mandrenguyen can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
enable nano |
|
please test |
|
+1 Size: This PR adds an extra 36KB to repository Comparison SummarySummary:
NANO Comparison SummarySummary:
Nano size comparison Summary:
|
|
+1 |
|
type egamma |
|
Since this is intended for the Run2 re-Nano in v15, @cms-sw/xpog-l2 can let us know if a backport to 150X is needed. I'm not 100% sure which release will be used. |
| recHitCollectionEE = 'reducedEgamma:reducedEERecHits' | ||
| ) | ||
|
|
||
| # This is now the same configuration as the one used in the Egamma regression v3 in 106XUL |
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.
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.
Hi @hqucms these workflows have changes because they are using the --era Run3_2024 --procModifiers run2_miniAOD_UL_preSummer20 modifier. This activates the calibration of patElectron which has been changed (corrected) by the updated configuration. What's the reason for this combination?
Effectively that configuration is trying to calibrate Run3 MC with Run2 scale&smearing, which is wrong. Anyway, the calibrated energy is not replacing the nominal and it is just stored in miniAOD as an additional calibEle* userFloat. In EGM we are not recommending to use calibrated quantity directly in Run3, but to reapply energy calibrations a-posteriori. So, in summary, this is not a problem, but for sure it is kind of ugly.
A backport to 150 will be needed. |
|
please test |
|
+1 Size: This PR adds an extra 16KB to repository Comparison SummarySummary:
NANO Comparison SummarySummary:
Nano size comparison Summary:
|
|
Hi @hqucms, @ftorrresd some Run3 workflows have changes because they are using the |
|
Hi @valsdav -- I think the changes are not caused by the (mis)use of |
|
Hi @hqucms. Let me summarize our findings.
The cff is only loaded in a function here: reducedEgammaTools, which is only loaded for this modifier So we don't understand how this should leak in the Run3 configuration. On the physics side, this should not be used anywhere in Run3, where the ECAL-trk parameters configuration are defined alongside the regression here, and the re-calibration is not applied neither in Mini or Nano (as intended). |
|
cmsDriver of the 2500.271 workflow: |
Hi @valsdav -- thanks a lot for the detailed explanation! Now I understand what is happening, and it's indeed caused by the misuse of the So in principle everything should be fine to proceed with the integration of this PR. I will make another PR to fix the misuse of the |
|
+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. @mandrenguyen, @antoniovilela, @rappoccio, @sextonkennedy (and backports should be raised in the release meeting by the corresponding L2) |
|
+1 |
PR description:
PR validation:
Tested using #48176 (comment)
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:
This PR is intended to be used for the Run2 reprocessing and therefore will require a backport to 15_0_X.