Update TICL & HGCal simulation truth validation to use new simulation truth collections, and simplify validation to use only SimCluster dataformat#51047
Open
tcuisset wants to merge 3 commits into
Conversation
Contributor
|
cms-bot internal usage |
Contributor
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-51047/49491
|
Contributor
|
A new Pull Request was created by @tcuisset for master. It involves the following packages:
@Martin-Grunewald, @Moanwar, @civanch, @cmsbuild, @ctarricone, @gabrielmscampos, @jfernan2, @kpedro88, @mandrenguyen, @mdhildreth, @mmusich, @rseidita, @srimanob can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
Contributor
|
@cmsbuild please test |
Contributor
|
@cmsbuild, please abort |
Contributor
|
test parameters:
|
Contributor
|
@cmsbuild, please test |
Contributor
|
-1 Failed Tests: RelVals Failed RelVals |
cmsbuild
added
tests-rejected
tests-pending
and removed
tests-started
tests-rejected
code-checks-approved
labels
Contributor
|
-1 Failed Tests: RelVals Failed RelVals |
Contributor
Author
Seems the failure is unrelated to this PR, see #51004 (comment) |
Contributor
Author
|
Is it possible to rerun the tests on this to get the comparisons, now that the relvals in IB are fixed ? |
Contributor
|
please test |
Contributor
|
-1 Failed Tests: RelVals-INPUT Failed RelVals-INPUT
Expand to see more relval errors ...
Comparison SummarySummary:
Max Memory Comparisons exceeding threshold@cms-sw/core-l2 , I found 2 workflow step(s) with memory usage exceeding the error threshold: Expand to see workflows ...
|
Contributor
|
Milestone for this pull request has been moved to CMSSW_20_0_X. Please open a backport if it should also go in to CMSSW_17_0_X. |
Contributor
|
(I assume this PR does not need a backport to 17_0_X (Run 3 legacy)) |
Contributor
yes |
CaloParticle dataformat is not used anymore, replaced with new collection of "CaloParticle" SimClusters Associators with CaloParticle are removed (SimCluster associators are used for the "CaloParticle" SimCluster collection) TICL validation code is updated in consequence SimTrackstersProducer is split into two producers : SimTrackstersProducers (only for simtracksters) and SimTICLCandidateProducer (produces SimTICLCandidates)
Contributor
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-51047/49663
|
Contributor
|
Pull request #51047 was updated. @Martin-Grunewald, @Moanwar, @civanch, @cmsbuild, @ctarricone, @gabrielmscampos, @jfernan2, @kpedro88, @mandrenguyen, @mdhildreth, @mmusich, @rseidita, @srimanob can you please check and sign again. |
Contributor
|
@cmsbuild please test |
Contributor
Author
|
I have rebased the PR to fix the conflict with the merging of the dataformats evolution.
also since we see no differences in the "allTracksterToTracksterAssociatorsByHits/ByLCs" with CaloParticle this would point towards an issue with the old LCtoCP associator rather than something related to CaloParticles themselves. |
Contributor
|
+1 Size: This PR adds an extra 32KB to repository Comparison SummarySummary:
|
Contributor
|
Milestone for this pull request has been moved to CMSSW_20_1_X. Please open a backport if it should also go in to CMSSW_20_0_X. |
kpedro88
reviewed
Jun 9, 2026
| return int(scToCpMap.at(simTS.seedIndex()).index()); | ||
| } | ||
| }; | ||
| auto getCPId = |
Contributor
There was a problem hiding this comment.
this function appears to be exactly repeated from HGVHistoProducerAlgo.cc. Would it make sense to designate a separate shared utility file to host this (and any other shared) function?
Contributor
Author
There was a problem hiding this comment.
It would definitely be a good idea. Then more generally the entire BarrelVHistoProducerAlgo is nearly entirely copied code from HGVHistoProducerAlgo, and same for BarrelValidator wrt HGCalValidator. So I would rather let TICL-barrel people deal with it.
Contributor
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-51047/49696
|
Contributor
|
Pull request #51047 was updated. @Martin-Grunewald, @Moanwar, @civanch, @cmsbuild, @ctarricone, @gabrielmscampos, @jfernan2, @kpedro88, @mandrenguyen, @mdhildreth, @mmusich, @rseidita, @srimanob can you please check and sign again. |
Contributor
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-51047/49697
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
9 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR description:
This is a follow-up on #50578, which had updated CaloTruthAccumulator to produce multiple SimCluster collections, with the intent to use only one dataformat for calorimeter truth information (instead of CaloParticle / SimCluster dataformat dichotomy).
This PR updates the TICL/HGCal validation to use the new SimCluster collections produced in CaloTruthAccumulator.
Use of CaloParticle dataformat is replaced nearly everywhere with the use of caloparticles in SimCluster dataformat (with just some remaining usage related to simTime).
Summary of the setup after this PR:
Documentation
Slides at TICL meeting
Slide 7 of HGCAL presentation at NGT MC truth workshop
Details:
PR validation:
Tested with various Phase 2 workflows:
There should not be any validation differences in reco objects. Very minor differences in validation could be caused by the switch from "legacy" to "boundary" SimCluster (which could change slightly validation results for status=1 photons that do not convert before reaching the calorimeter)
FYI @felicepantaleo @waredjeb