Hcal depth energy fraction packed candidates value map #26465
Conversation
|
The code-checks are being triggered in jenkins. |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-26465/9294
|
|
@cmsbuild please test |
|
The tests are being triggered in jenkins. |
|
A new Pull Request was created by @hatakeyamak (Kenichi Hatakeyama) for master. It involves the following packages: PhysicsTools/PatAlgos @perrotta, @cmsbuild, @santocch, @slava77 can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
FYI: I used this very simple python file to check this addition. |
|
Comparison job queued. |
|
@cmsbuild please test |
|
The tests are being triggered in jenkins. |
|
Comparison job queued. |
|
Comparison is ready Comparison Summary:
|
|
Here is a plot from JetHT2018C wf 136.876
It looks like the setup works OK. |
|
Good. I also checked locally for one data workflow before I made the last commit. So, I guess it looks good to sign off? |
| source="std::vector<uint8_t> fractionsI_" | ||
| target="fractions_"> | ||
| <![CDATA[ newObj->initFloatVector(); | ||
| ]]> |
There was a problem hiding this comment.
Static analyzer picked up on this line
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-26465/34296/llvm-analysis/report-9a8c17.html#EndPath
According to it, the code for this rule looks like the following
pat::HcalDepthEnergyFractions* newObj = (pat::HcalDepthEnergyFractions*)target;
if (oldObj) {}
if (newObj) {}
newObj->initFloatVector();
@pcanal @Dr15Jones
By some magic (?) I would expect that the body of CDATA[ should end up inside if (newObj) {.
Are we using the syntax for ioread incorrectly or should we simply add if (newObj) { ?
We have the same pattern in a few other ioread rules in CMSSW. So, a more general solution is perhaps outside the scope of this PR.
There was a problem hiding this comment.
That code is there solely to prevent unused variable warning. It needs to be updated to be (void)newObj, etc.
|
+1
|
|
+1 |
|
merge |
|
+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 be automatically merged. |
cmsbuild
added
analysis-approved
fully-signed
and removed
analysis-pending
pending-signatures
labels
PR description:
Includes HCAL depth information for packed candidates in MINIAOD for high-level object use (e.g. PUPPI recomputation at the post ultra-legacy MINIAOD stage). The promising results are obtained [1,2], but needs more time to finalize it. It means it is important to have this information in packedCandidates, so that the PUPPI weights can be recomputed on-the-fly from MINIAOD.
[0] https://github.com/cms-sw/cmssw/files/3056535/he_depth_reco.pdf
[1] https://indico.cern.ch/event/799365/contributions/3333038/attachments/1805670/2946660/pre.pdf
[2] https://indico.cern.ch/event/799365/contributions/3339621/attachments/1805694/2946705/JK_JetMET_03_19_2.pdf
If this ValueMap-based implementation as suggested by @slava77 and @peruzzim looks good, this may supersede:
[3] #26363
Now. I also use vectors instead of arrays to store various depth information.
@peruzzim @fgolf @zdemirag @ahinzmann @bendavid @mariadalfonso
PR validation:
Made sure that the this information gets added to MINIAOD as intended. The size increase in TTbar sample with PU (tested in 2000 events) in 2017 & 2018 scenarios is
a little larger increase than what [3] gives for 2018 (2.7%) but very similar.
if this PR is a backport please specify the original PR:
This is not a backport.