HGCAL RecHits Calibration for V16 geometry scenario #36728
Conversation
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36728/27827
|
|
A new Pull Request was created by @apsallid for master. It involves the following packages:
@clacaputo, @cmsbuild, @AdrianoDee, @srimanob, @slava77, @jpata can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
@cmsbuild please test |
| @@ -2,6 +2,60 @@ | |||
| from SimCalorimetry.HGCalSimProducers.hgcalDigitizer_cfi import * | |||
| from RecoLocalCalo.HGCalRecProducers.HGCalUncalibRecHit_cfi import * | |||
|
|
|||
| from Configuration.Eras.Modifier_phase2_hgcalV12_cff import phase2_hgcalV12 | |||
|
|
|||
| def calcWeights(weightsPerLayer): res = [sum(wei)/2. for wei in zip(weightsPerLayer[:], weightsPerLayer[1:] + [weightsPerLayer[0]])]; res[0] = 0.0; res[-1] = res[-2]; return res; | |||
There was a problem hiding this comment.
Would it make sense to call the first weight as dummy_weight, to make it clear that it is associated with a non-existing layer?
That would also make the formula a little more explicit by using that value instead of weightsPerLayer[0], to make it clear that you are also adding at the end a dummy value, not the first weight, that would make no sense.
The same would be true for res[0] = dummy_weight as well.
Actually, you could directly add [weightsPerLayer[-1]] as the last element and remove the res[-1] = res[-2], to make it explicit that the last layer has no mean.
There was a problem hiding this comment.
I'm still convinced that extending the second array with the last element, and not with the first, dummy one, would make the code easier and remove the necessity of setting res[-1] manually, no?
| @@ -4,6 +4,8 @@ | |||
|
|
|||
| fCPerMIP_v10 = cms.vdouble(2.06,3.43,5.15) #120um, 200um, 300um | |||
|
|
|||
| fCPerMIP_v16 = fCPerMIP_v10 | |||
There was a problem hiding this comment.
I do not think there's a need to version the fCPerMIP. Would it make sense to just reuse fCPerMIP_v10?
Or, even better, remove the versioning from the variable's name completely?
I believe that could also imply removing the last four lines you have added, right?
There was a problem hiding this comment.
@rovere The default values are with the MPV, while the ones with the version are with the mean. Isn't it better to remove the versioning and introduce two variables (fCPerMIP_mean, fCPerMIP_mpv) and in each modifier use the appropriate one?
There was a problem hiding this comment.
@apsallid absolutely yes, that would be the optimal choice indeed!
cmsbuild
added
code-checks-pending
tests-pending
and removed
tests-started
code-checks-approved
labels
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36728/27834
|
|
Pull request #36728 was updated. @clacaputo, @cmsbuild, @AdrianoDee, @srimanob, @slava77, @jpata can you please check and sign again. |
|
@cmsbuild please test |
|
Pull request #36728 was updated. @clacaputo, @cmsbuild, @AdrianoDee, @srimanob, @slava77, @jpata can you please check and sign again. |
|
@cmsbuild please test |
|
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-c35101/22069/summary.html Comparison SummarySummary:
|
|
@apsallid, the change that you have proposed seems to have fixed the
|
|
@clacaputo You cannot imagine how glad I am! |
|
+Upgrade Re-Sign. |
|
+reconstruction
|
cmsbuild
added
fully-signed
reconstruction-approved
and removed
reconstruction-pending
pending-signatures
labels
|
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. @perrotta, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2) |
|
|
+1
|
PR description:
The HGCAL V16 geometry is substantially different than previous versions with altered
longitudinal structure. In this PR we update the dEdx weights and regional e/m factors
so that reconstructed hits are calibrated. With respect to previous work, here we inject weights
per layer, so that they are useful also to both offline and L1T worlds.
PR validation:
In these slides we have validated that this PR works as expected.
if this PR is a backport please specify the original PR and why you need to backport that PR:
This is not a backport.
@rovere @pfs @cseez @felicepantaleo @ebrondol @lecriste