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
HGCal - bugfix - removing energy of out-of-range hits #20479
HGCal - bugfix - removing energy of out-of-range hits #20479
Conversation
The code-checks are being triggered in jenkins. |
+code-checks |
A new Pull Request was created by @felicepantaleo (Felice Pantaleo) for master. It involves the following packages: RecoParticleFlow/PFClusterProducer @perrotta, @cmsbuild, @slava77 can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
type bugfix |
assign upgrade |
New categories assigned: upgrade @kpedro88 you have been requested to review this Pull request/Issue and eventually sign? Thanks |
The code-checks are being triggered in jenkins. |
It's good to go now. |
please test |
+code-checks |
The tests are being triggered in jenkins. |
+1 The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic: |
Comparison job queued. |
Comparison is ready Comparison Summary:
|
In the PR description:
it looks like the calibration was added already. |
@slava77 done |
+1 |
@felicepantaleo @predragm |
@@ -2,5 +2,5 @@ | |||
|
|||
minEtaCorrection = cms.double(1.4) | |||
maxEtaCorrection = cms.double(3.0) | |||
hadronCorrections = cms.vdouble(1.16, 1.10, 1.08, 1.08, 1.09, 1.11, 1.14, 1.26) | |||
egammaCorrections = cms.vdouble(1.00, 1.00, 1.00, 1.00, 1.02, 1.03, 1.03, 1.03) | |||
hadronCorrections = cms.vdouble(1.24, 1.24, 1.24, 1.23, 1.24, 1.25, 1.29, 1.29) |
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.
is it clear how the supposedly minor effect of the fix is affecting the hadron correction so significantly.
Some eta bins are up by 15% (compared to at most 1% for egamma corrs).
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.
the energy of the hits outside the radius is not accounted anymore, so it's not a minor effect for hadrons.
clusters from 50GeV pions have usually a radius of 10-ish cm (in FH), and are much smaller (2-3cm) in EE.
One could probably set two radii in the future depending on the subdetector...
+1
Given absence of a backport to 93X, it's unclear if this is required there or can stay in 94X. |
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. @davidlange6, @slava77, @smuzaffar (and backports should be raised in the release meeting by the corresponding L2) |
+1 |
This PR is a bug fix and should solve the problem in #20458
Hits which are outside the 10cm radius from the center of gravity are removed by setting their fraction to zero, but their energy was never subtracted from the energy of the cluster.
This is a problem as the final energy of the PFCluster is not computed as the sum of the energy of the hits * their fraction, but it gets assigned the energy of the realistic cluster.
Recalibration is also included
@rovere @kpedro88