Skip to content
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

Calo particle validation with graph #22596

Merged
merged 28 commits into from Apr 3, 2018

Conversation

rovere
Copy link
Contributor

@rovere rovere commented Mar 13, 2018

REPLACEMENT OF #21701

While doing work on HGCAL, I stumbled upon the CaloParticles several times. In order to "learn&understand" what they are and how they are produced, I took a look at the CaloTruthAccumulator, which is the package responsible for their creation. I found the code to be convoluted and overly complicated. I decided to rewrite it from scratch, keeping the same functionalities using graphs, since particle decays are "naturally" described by graph (and their algorithms).

This is what I came up with: I believe this code is more compact, clean and understandable with respect to the original one.

I slightly changed the meaning of the embedded simClusters for CaloParticles that decay within a certain distance from HGCAL, since in the previous implementation all children were artificially collapsed under the original particle, while now they are added one-by-one as children with their own simClusters.

Besides this change, from quick tests on single particle guns (Zee), I did not notice significant differences. While I trust the algorithm I wrote, I'm not sure the old one was fully capturing all simClusters at all levels of the decay chain. So I do not exclude that there won't be any regression.

Actually the debug package contains also a complete reimplementation of
the CaloParticle logic fully based on the BGL that greatly simplify the
extremely complex logic of the currently used CaloTruthAccumulator. The
implementation is not yet final but gives already identical results if
compared with the regularly produced CaloParticles.
Full recursion and creation of CaloParticles.
Add more histograms to monitor CaloParticles and monitor more kind of
particles based on their pdgId.
Few particles  are linked still to vertex 0 but are separate from the
main vertex graph. This will recover them and promote them to be
CaloParticles if them or any of their children leave hits in the
configured calorimeters.
Every CaloParticle is linked to the first stable particle originating
the cascade of particles that left hits in the calorimeters: that
stable particle is not included as a simCluster (unless it itself left
hits in the calorimiters). Additional code has been added to find a
matching track based on the trackId linked to the CaloParticle itself,
not to the associated simClusters. The most important clients affected
by this change are electrons.
@cmsbuild
Copy link
Contributor

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-22596/26849/summary.html

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 2066 differences found in the comparisons
  • DQMHistoTests: Total files compared: 29
  • DQMHistoTests: Total histograms compared: 2477528
  • DQMHistoTests: Total failures: 310
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2477042
  • DQMHistoTests: Total skipped: 176
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 9732.21 KiB( 23 files compared)
  • Checked 118 log files, 9 edm output root files, 29 DQM output files

@rovere
Copy link
Contributor Author

rovere commented Mar 14, 2018

In order to speed-up integration, could someone suggest the best way to have the new-package-pending approved?

@kpedro88
Copy link
Contributor

The bot gives instructions for this:

The following packages do not have a category, yet:

SimGeneral/Debugging
Please create a PR for https://github.com/cms-sw/cms-bot/blob/master/categories_map.py to assign category

@rovere
Copy link
Contributor Author

rovere commented Mar 15, 2018

@kpedro88
done in cms-sw/cms-bot#963

@fabiocos
Copy link
Contributor

@kpedro88 @rovere when the PR is approved and merged, cms-bot will be updated accordingly (as we have done for other packages recently)

@civanch
Copy link
Contributor

civanch commented Mar 19, 2018

+1

@dmitrijus
Copy link
Contributor

+1

@rovere
Copy link
Contributor Author

rovere commented Mar 21, 2018

Hi all,
I'd like to have this included in 10_1_0, if possible.
Who's responsible for the reconstruction signature, in this case? @slava77 @perrotta or @kpedro88 ??

@perrotta
Copy link
Contributor

+1

  • Mostly sim and validation, meant to Phase2
  • Changes in RecoParticleFlow/PFSimProducer/plugins/SimPFProducer.cc modify the sim/reco flow for HGCal, with some minor effect on reconstructed quantities (Calo particle validation with graph #22596 (comment)), which is as expected

@cmsbuild
Copy link
Contributor

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, @fabiocos (and backports should be raised in the release meeting by the corresponding L2)

@smuzaffar smuzaffar modified the milestones: CMSSW_10_1_X, CMSSW_10_2_X Mar 29, 2018
@fabiocos
Copy link
Contributor

@rovere are we expecting more stuff entering into the SimGeneral/Debugging package? At present it is just one class. For the rest the PR is ok for me

@fabiocos
Copy link
Contributor

fabiocos commented Apr 3, 2018

after a discussion with @rovere, this will be possibly reviewed in a forthcoming development in case no further additions are needed

@fabiocos
Copy link
Contributor

fabiocos commented Apr 3, 2018

+1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants