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

Additon of StFwdTrack to StEvent, updates to StFwdTrackMaker to accomodate #492

Merged
merged 54 commits into from Apr 19, 2023

Conversation

jdbrice
Copy link
Contributor

@jdbrice jdbrice commented Feb 2, 2023

This PR replaces #397 (it was easier to start fresh)
Major updates

  • New StEvent StFwdTrack model that does not depend on GenFit
  • Updates to FwdTracker:
    • Add mapped BField to speed up tracking in forward region
    • Add Generalized broken lines refit for alignment studies
    • Add option for tracking in zero field running
    • Updates for new sTGC geometry
    • Write new StFwdTracks to StEvent
    • Macros and READMEs for testing and running on simulation / DAQ / StEvent files

@jdbrice
Copy link
Contributor Author

jdbrice commented Feb 3, 2023

This PR incorporates the discussion that we had over several S&C meetings about the StFwdTracks. Specifically it:

  • Removes dependency on any external libraries (GenFit)
  • Stores only enough information for generic physics analysis + ability to rebuild GenFit track for advanced physics use cases
  • As discussed, currently the only "extra" are the forward track projections to internal tracking detectors (PV, FST, FTT). These could be dropped for official productions to reduce the data size. In contrast the projections to EPD, ECAL and HCAL are needed for normal physics uses cases.

@jdbrice
Copy link
Contributor Author

jdbrice commented Feb 6, 2023

commit 0d35ff2
uses the cached STAR geometry in a ROOT file instead of loading a local file.
This should resolve the blocking issue on PR #410

@plexoos plexoos requested a review from veprbl as a code owner February 7, 2023 23:19
@starsdong
Copy link
Member

Dear All, Daniel made quite great efforts in addressing the comments and suggestions. Would it be possible for reviewers to take a look and see whether we can proceed with this PR? Thanks

@jdbrice
Copy link
Contributor Author

jdbrice commented Apr 14, 2023

@starsdong @plexoos
Any comments that need to be addressed before we resolve this PR?

@starsdong
Copy link
Member

I went through the updates from Daniel and I think all the comments are addressed and we are ready to merge. We wait until COB of Monday, in case anyone wants to bring up anything. If no further comment, Dmitri, let us merge this PR. Thanks

@plexoos
Copy link
Member

plexoos commented Apr 18, 2023

I just fixed a conflict affecting a test using the FwdTrack option. Hopefully, it passes and we can merge 🤞

@plexoos
Copy link
Member

plexoos commented Apr 18, 2023

The code does not compile due to the following error:

#10 1073.1 g++ -m64 -fPIC -pipe -Wall -Woverloaded-virtual -std=c++0x -Wno-long-long -pthread -Wno-deprecated-declarations -Werror -O2 -g -falign-loops -falign-jumps -falign-functions -Dsl79_gcc485 -D__ROOT__ -DNEW_DAQ_READER -I. -IStRoot -I.sl79_gcc485/include -I/opt/software/linux-scientific7-x86_64/gcc-4.8.5/root-5.34.38-l3v6vso6qgojm4l2ctwjojs6trbt4hpn/include -c .sl79_gcc485/OBJ/StRoot/StFcsTrackMatchMaker/StFcsTrackMatchMaker.cxx -o .sl79_gcc485/OBJ/StRoot/StFcsTrackMatchMaker/StFcsTrackMatchMaker.o
#10 1073.8 .sl79_gcc485/OBJ/StRoot/StFcsTrackMatchMaker/StFcsTrackMatchMaker.cxx: In destructor 'virtual StFcsTrackMatchMaker::~StFcsTrackMatchMaker()':
#10 1073.8 .sl79_gcc485/OBJ/StRoot/StFcsTrackMatchMaker/StFcsTrackMatchMaker.cxx:62:12: error: deleting object of polymorphic class type 'StEpdGeom' which has non-virtual destructor might cause undefined behaviour [-Werror=delete-non-virtual-dtor]
#10 1073.8      delete mEpdgeo;
#10 1073.8             ^
#10 1073.9 cc1plus: all warnings being treated as errors
#10 1073.9 cons: *** [.sl79_gcc485/OBJ/StRoot/StFcsTrackMatchMaker/StFcsTrackMatchMaker.o] Error 1

A quick look reveals that this class https://github.com/star-bnl/star-sw/blob/main/StRoot/StEpdUtil/StEpdGeom.h includes the ClassDef macro and thus defines virtual methods. However, it does not define a virtual destructor which is required in this case. One way to fix this is to make StEpdGeom inherit from TObect

plexoos added a commit that referenced this pull request Apr 18, 2023
This type is neither an StMaker nor an IO class so, using ClassDef is
not critical. On the contrary, it causes an error due to missing virtual
destructor when included in compiled code. E.g. see
#492
This type is neither an StMaker nor an IO class so, using ClassDef is
not critical. On the contrary, it causes an error due to missing virtual
destructor when included in compiled code. E.g. see
star-bnl#492
plexoos added a commit that referenced this pull request Apr 19, 2023
This type is neither an StMaker nor an IO class so, using ClassDef is
not critical. On the contrary, it causes an error due to missing virtual
destructor when included in compiled code. E.g. see
#492
@plexoos plexoos merged commit 9b0dffc into star-bnl:main Apr 19, 2023
156 checks passed
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.

None yet

3 participants