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 Geant track info (g2t) to StFcsHit #379

Merged
merged 8 commits into from
Sep 6, 2022
Merged

Conversation

akioogawa
Copy link
Contributor

Adding parent g2t track info to StEvent/StFcsHit.
Update StFcsFastSimulatorMaker to push these info from g2t table to StFcsHit.
Adding utility functions to trace back to immediate parent and primary g2t track for a given StFcsHit and StFcsCluster in StFcsDb.

StRoot/StEvent/StFcsHit.cxx Outdated Show resolved Hide resolved
StRoot/StEvent/StFcsHit.h Show resolved Hide resolved
StRoot/StFcsClusterMaker/StFcsClusterMaker.cxx Outdated Show resolved Hide resolved
StRoot/StFcsDbMaker/StFcsDb.h Outdated Show resolved Hide resolved
@klendathu2k
Copy link
Contributor

klendathu2k commented Jul 29, 2022 via email

@plexoos
Copy link
Member

plexoos commented Jul 29, 2022

@akioogawa can you elaborate on how the new data member StFcsHit::mGeantTracks will be used? It looks like you want to make it persistent in event.root files but I don't think we normally save them in simulation jobs.

@akioogawa
Copy link
Contributor Author

A student is working on cluster finder for charged hadrons at StEvent file right now. Once this is in the place, Daniel will be adding this to Mudst, and we move on to Mudst.

Copy link
Contributor

@klendathu2k klendathu2k left a comment

Choose a reason for hiding this comment

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

A couple of small changes requested, but otherwise this looks good to me.

StRoot/StEvent/StFcsHit.cxx Outdated Show resolved Hide resolved
StRoot/StFcsDbMaker/StFcsDb.cxx Outdated Show resolved Hide resolved
StRoot/StFcsDbMaker/StFcsDb.h Outdated Show resolved Hide resolved
StRoot/StFcsDbMaker/StFcsDb.cxx Outdated Show resolved Hide resolved
StRoot/StFcsDbMaker/StFcsDb.cxx Outdated Show resolved Hide resolved
StRoot/StFcsFastSimulatorMaker/StFcsFastSimulatorMaker.cxx Outdated Show resolved Hide resolved
@klendathu2k klendathu2k requested a review from genevb August 2, 2022 16:51
StRoot/StEvent/StFcsHit.h Outdated Show resolved Hide resolved
@@ -87,9 +87,13 @@ class StFcsHit : public StObject {
void setCluster(StFcsCluster* clu) {mCluster = clu;}
StFcsCluster *cluster() {return mCluster;}

const vector<pair<unsigned int, float>>& getGeantTracks() const {return mGeantTracks;}
Copy link
Member

Choose a reason for hiding this comment

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

Do we really need this information? It's not complete enough to learn something about the shower, but too complicated to learn about what caused the hit. I don't see why the analyzers couldn't get by with just unsigned int idTruth() and float qaTruth() (aka fraction in your current PR). This should be enough for two-particle studies (energy fraction that is not associated with the leading particle must be coming from the other one). I suppose, unlike for TPC, you also wanted to store the particle that arrived to the detector (getParentG2tTrack) in the current PR, but again, this is not equivalent to storing energy and id of every hit.

I'm mostly asking because I'm pretty sure the interface from StEvent is going to migrate to MuDst without a change.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, it is needed. A single ID truth is insufficient to understand the relative contributions of electromagnetic and hadronic components to the cluster energy response (You don't know what the "rest" of the energy is...)

With the list of track IDs hitting each cluster, their contribution to the energy deposition in the cluster, and the MC track's kinematics from the MC event record, you have the complete information needed to understand the clustering performance.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, it is needed. A single ID truth is insufficient to understand the relative contributions of electromagnetic and hadronic components to the cluster energy response (You don't know what the "rest" of the energy is...)

Okay, but if that is the case, I need to also save PDG id. If not, I need to look into geant.root, and at this point I might as well not save anything to event.root. Right?

With the list of track IDs hitting each cluster, their contribution to the energy deposition in the cluster, and the MC track's kinematics from the MC event record, you have the complete information needed to understand the clustering performance.

Again, for any foreseeable study, there are just two particles that could merge into the same cluster, one fraction per cell describes that case it well enough.

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay, but if that is the case, I need to also save PDG id. If not, I need to look into geant.root, and at this point I might as well not save anything to event.root. Right?

PDG ID is available in the MuDst MuMcTrack branch.

Again, for any foreseeable study, there are just two particles that could merge into the same cluster, one fraction per cell describes that case it well enough.

Relative contribution of hadronic and electromagnetic particles to the energy of a cluster is one such study which requires this information.

Copy link
Member

Choose a reason for hiding this comment

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

PDG ID is available in the MuDst MuMcTrack branch.

I'm not very familiar with that facility. And this answer also implies that this structure will be propagated to MuDst.

Again, for any foreseeable study, there are just two particles that could merge into the same cluster, one fraction per cell describes that case it well enough.

Relative contribution of hadronic and electromagnetic particles to the energy of a cluster is one such study which requires this information.

Okay, so that would be a study that looks at cluster merging and hadronic/electromagnetic fractions at the same time. Thank you for the explanations. I hope other experts/analyzers can chime in.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  • Yes, idTruth is not enough, especially when many towers contributing to a cluster, and hadron showers are fat and big.
  • Yes, when we get to mudst, since Mudst has StMcTrack, but not g2t_hits.
  • No, when reading event.root file and geant.root file in sync. But this means we essentially re-run StFcsFastSimulationMake and creating new StFcsHits. (for historical reason, we don't have StMcTrack in StEvent).

return &g2ttrk[primary[order].first-1];
}

const g2t_track_st* StFcsDb::getParentG2tTrack(StFcsCluster* c, g2t_track_st* g2ttrk, float& fraction, int& ntrk, unsigned int order){
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it would be good to document somewhere what order means in these functions. It appears to me that invoking these functions with different values of order will be mean repeating all the code as nothing about the parents is stored/cached, so I'd really like to better understand the use cases of order to understand if this is the best way to encode these functions.

Thanks,
-Gene

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  • It is in commet in StFcsDb.h file
  • With order=0 returns the highest contributing g2t_track_id
  • With order=1 returns the 2nd highest contributing g2t_track_id
  • Yes, I can make sorted vector available to user....But then user need to delete them.
  • Or cache sorted results somehow, but then delete when & where?
  • Any suggestions will be welcomed

-  const taken out from passing by value

StFcsDb
- "using namespace std;" added
- doxygen-style (///) comment, including what "order" means
- std::sort => std::nth_element
StRoot/StFcsDbMaker/StFcsDb.cxx Outdated Show resolved Hide resolved
StRoot/StFcsDbMaker/StFcsDb.cxx Outdated Show resolved Hide resolved
Make use of getG2tTrack for StFcsHit as well using dummy cluster
Changing to range loop instead of index loop in getG2tTrack.
@akioogawa
Copy link
Contributor Author

I adopted your idea of fake cluster and also dropped auto sorting in StFcsFastSimulagtorMaker, leaving all sorting done in StFcsDb utility function at analysis stage.

@plexoos plexoos added this to the SL22c milestone Aug 29, 2022
Copy link
Member

@plexoos plexoos left a comment

Choose a reason for hiding this comment

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

Thank you for addressing the comments. There is only one last suggestion from my side in the comments, I'll leave it up to you.

StRoot/StFcsDbMaker/StFcsDb.cxx Show resolved Hide resolved
@plexoos plexoos changed the title Adding g2t track info to StFcsHit Add Geant track info (g2t) to StFcsHit Aug 29, 2022
Copy link
Contributor

@klendathu2k klendathu2k left a comment

Choose a reason for hiding this comment

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

It looks to me that all of my concerns were addressed. Approving.

@akioogawa
Copy link
Contributor Author

Just did merge main into this branch. Any other comments?

@plexoos
Copy link
Member

plexoos commented Sep 1, 2022

Hi Daniel, looks like we need your approval to include this change in the next SL22c release

@plexoos plexoos merged commit ec3fb16 into star-bnl:main Sep 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants