Skip to content

Conversation

@AuroraPerego
Copy link
Contributor

PR description:

Currently the idLastStoredAncestor() in the track info stores:

  • the id of the G4 track itself if it is marked as to be saved in the simTrack collection
  • the id of the last parent saved in the simTrack collection if the track itself will not be saved

This led to a problem in the case of a particle which is saved in the collection but its parent is not, because idLastStoredAncestor() pointed to that track, but there was no way in the final simtrack collection to understand from which particle this track originated.
With this change the idLastStoredAncestor() will contain:

  • the id of the G4 track itself if it is marked as to be saved in the simTrack collection, and all its parents up to the first G4 track are saved
  • the id of the last parent saved in the simTrack collection otherwise

As an example, suppose we have track 1 -> track 2 -> track 3 -> track 4 ( a -> b means that track a is the mother of track b) and track 3 is the only one not saved in the event. Before this PR track 4 was saved but there was no way to link it to track 1, now it stores the Geant4 Id of track 2.

This is a follow-up of #47883 that eventually will allow us to recover the simClusters that are currently discarded.

PR validation:

Looked at the output files of the SimGeneral/Debugging/python/caloParticleDebugger_cfg.py, the only expected change is in the value returned by the method getPrimaryOrLastStoredID() of the simTrack.

FYI @rovere @fabiocos

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 24, 2025

cms-bot internal usage

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @AuroraPerego for master.

It involves the following packages:

  • SimDataFormats/Track (simulation)
  • SimG4CMS/Calo (simulation)
  • SimG4CMS/Forward (simulation)
  • SimG4CMS/Muon (simulation)
  • SimG4CMS/Tracker (simulation)
  • SimG4Core/Application (simulation)
  • SimG4Core/CustomPhysics (simulation)
  • SimG4Core/Notification (simulation)
  • SimGeneral/Debugging (simulation)

@civanch, @cmsbuild, @kpedro88, @mdhildreth can you please review it and eventually sign? Thanks.
@ReyerBand, @VinInn, @VourMa, @apsallid, @bsunanda, @denizsun, @fabiocos, @felicepantaleo, @makortel, @martinamalberti, @missirol, @mmusich, @mtosi, @rovere, @salimcerci, @slomeo, @thomreis, @wang0jin, @youyingli this is something you requested to watch as well.
@ftenchini, @mandrenguyen, @sextonkennedy you are the release manager for this.

cms-bot commands are listed here

@civanch
Copy link
Contributor

civanch commented Sep 24, 2025

@AuroraPerego , it would be good to have a discussion on this modifications. We would like to understand how this modification affect Run2 and Run3 WFs? How proposed modification affect future asynchronous approach for Phase2 WFs, when track will be handled in parallel?

@kpedro88
Copy link
Contributor

Another question: does this change impact the fineCalo approach for MC truth?

@AuroraPerego
Copy link
Contributor Author

@AuroraPerego , it would be good to have a discussion on this modifications.

Sure

We would like to understand how this modification affect Run2 and Run3 WFs?

Another question: does this change impact the fineCalo approach for MC truth?

The idLastStoredAncestor_ in the TrackInformation is the only member I'm changing, it was introduced by Fabio for MTD hits classification and it's used only by him (as far as I know), so changing it shouldn't impact Run2/3 workflows or fineCalo.

How proposed modification affect future asynchronous approach for Phase2 WFs, when track will be handled in parallel?

If mother / daughter tracks aren't handled in parallel, there should be no problem. If they are, then the daughter should know if the mother is saved or not before filling this member, but I suppose it's a general problem here.

@civanch
Copy link
Contributor

civanch commented Sep 25, 2025

please test

1 similar comment
@civanch
Copy link
Contributor

civanch commented Sep 26, 2025

please test

@cmsbuild
Copy link
Contributor

+1

Size: This PR adds an extra 16KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-082fda/48290/summary.html
COMMIT: 2dce981
CMSSW: CMSSW_16_0_X_2025-09-26-1100/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/48986/48290/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

  • You potentially removed 2 lines from the logs
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 51
  • DQMHistoTests: Total histograms compared: 3911484
  • DQMHistoTests: Total failures: 35
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3911429
  • DQMHistoTests: Total skipped: 20
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 50 files compared)
  • Checked 218 log files, 188 edm output root files, 51 DQM output files
  • TriggerResults: no differences found

@civanch
Copy link
Contributor

civanch commented Sep 26, 2025

+1

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

@mandrenguyen
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit 691c816 into cms-sw:master Sep 28, 2025
10 checks passed
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.

5 participants