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

Add additional information to the SimTracks #46979

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

AuroraPerego
Copy link
Contributor

PR description:

Looking at the SimTracks and SimVertices in single pion events we found some "orphan" vertices that were triggering the creation of SimClusters not associated with any CaloParticle.
For example, when a particle enters HGCAL we stop keeping track of the history, if it does backscattering and enters the barrel calorimeters we lose the connection with the initial particle.
With this additional information, we can connect the orphan simClusters to the original particle that created them and flag those that come from backscattering.

This information will be used at the SimCluster level in the CaloTruthAccumulator in a follow-up PR.

Note: the behaviour of the already existing methods (genpartIndex() and noGenpart()) has not been changed to preserve backward compatibility and the new getPrimaryID() method is used to obtain the genParticle index.
The logic with the uint8_t instead of a list of bool reduces the class size and gives the possibility to add more flags / categories in the future.

PR validation:

tested on a single pion workflow enabling Geant4-related cout.

FYI @rovere @felicepantaleo @fabiocos

- is or not from backscattering
- is or not a primary
- the GenParticle ID is always saved if available and returned with a
specific method, while the old genpartIndex() returns -1 if it's not a
primary as before
@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 17, 2024

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)
  • SimG4Core/Application (simulation)
  • SimG4Core/Notification (simulation)

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

cms-bot commands are listed here

@rovere
Copy link
Contributor

rovere commented Dec 17, 2024

@cmsbuild please test

@cmsbuild
Copy link
Contributor

+1

Size: This PR adds an extra 32KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-2b226b/43509/summary.html
COMMIT: 774e317
CMSSW: CMSSW_15_0_X_2024-12-17-1100/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/46979/43509/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

private:
int ivert;
int igenpart;

math::XYZVectorD tkposition;
math::XYZTLorentzVectorD tkmomentum;

bool crossedBoundary_;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AuroraPerego this changes a member of a persistent object, how do we ensure backward compatibility here? How are we correctly interpreting all the old files? I do not see a rule in the classes_def.xml

@@ -88,6 +93,8 @@ class TrackWithHistory {
bool storeTrack_{false};
bool saved_{false};
bool crossedBoundary_{false};
bool isFromBackScattering_{false};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these additions seem to duplicate information already available in TrackInformation, where are they needed because they cannot be replaced by TrackInformation?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

answering to myself, apparently the SimTrackManager is based on TrackWithHistory at present

@@ -34,8 +34,9 @@ class SimTrack : public CoreSimTrack {
bool noVertex() const { return ivert == -1; }

/// index of the corresponding Generator particle in the Event container (-1 if no Genpart)
int genpartIndex() const { return igenpart; }
bool noGenpart() const { return igenpart == -1; }
bool isPrimary() const { return (trackInfo_ >> 1) & 1; }
Copy link
Contributor

@civanch civanch Dec 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AuroraPerego , track info defined in all cases, when tracking of given particle is started. If it is not true to me it is a bug. Primary particle if defined at start tracking.

int genpartIndex() const { return igenpart; }
bool noGenpart() const { return igenpart == -1; }
bool isPrimary() const { return (trackInfo_ >> 1) & 1; }
int genpartIndex() const { return isPrimary() ? igenpart : -1; }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would guess that it is not a good solution.

@civanch
Copy link
Contributor

civanch commented Dec 18, 2024

-1

we discussed MC truth during 2023-2024 and not do real change of MC truth, because we preserve Run3 results. Any change for Run3 is a problem.

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