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

Move filling of data classes (event/track/hits) into restG4 package instead of geant4lib #21

Closed
lobis opened this issue Jul 5, 2021 · 8 comments
Assignees

Comments

@lobis
Copy link
Member

lobis commented Jul 5, 2021

Currently the logic to insert a Geant4 event/track/step into a geant4lib data structure is implemented inside geant4lib via, for example:

void AddG4Hit(TVector3 pos, Double_t en, Double_t hit_global_time, Int_t pcs, Int_t vol, Double_t eKin,
                  TVector3 momentumDirection) {
        fHits.AddG4Hit(pos, en, hit_global_time, pcs, vol, eKin, momentumDirection);
    }

In my opinion this logic should be implemented in restG4, and the argument for these method should be a Geant4 class such as G4Track, G4Step etc. At first glance I think the best solution would be to implement a constructor that takes only the equivalent Geant4 structure as argument (TRestGeant4Track::TRestGeant4Track(const G4Track* track)), then we could place the tracks/steps into a container using std::vector emplace_back.

However to achieve this we would probably need to define in the restG4 package a derived class that implements this method/constructor, since obviously we do not want to link to Geant4 in geant4lib. Then we would cast the structure to the base class.

@lobis lobis self-assigned this Jul 5, 2021
@jgalan
Copy link
Member

jgalan commented Jul 5, 2021

I dont see the reason to migrate out things from geant4lib, I think restG4 should not be responsible to define our data structures, it should be geant4lib.

If it is that essential to include G4Track into geantlib it could still be done through a piece of code that is enabled/disabled if REST was linked or nor to restG4, and therefore the Geant4 official libraries. However, if we compile REST without restG4 we will not need to fill any Geant4 official object, then those features will not be made available.

@jgalan
Copy link
Member

jgalan commented Jul 5, 2021

We should also have the mind open to the possibility to integrate other particle physics simulation packages (fluka, heed, ...) using the same existing geant4lib structures, metadata classes and processes.

Even if it will be named geant4lib for historical reasons.

@lobis
Copy link
Member Author

lobis commented Jul 5, 2021

I dont see the reason to migrate out things from geant4lib, I think restG4 should not be responsible to define our data structures, it should be geant4lib.

If it is that essential to include G4Track into geantlib it could still be done through a piece of code that is enabled/disabled if REST was linked or nor to restG4, and therefore the Geant4 official libraries. However, if we compile REST without restG4 we will not need to fill any Geant4 official object, then those features will not be made available.

The data structure will be defined in geant4lib, the interface to fill this data structure will be defined in restG4, that is my idea.

For example, we could have some slim classes in restG4 such as:

class TRestGeant4StepsExtended : public TRestGeant4Hits {
   public:
    void InsertStep(const G4Step*);
};

class TRestGeant4TrackExtended : public TRestGeant4Track {
   public:
    TRestGeant4TrackExtended(const G4Track*);
};

class TRestGeant4EventExtended : public TRestGeant4Event {
   public:
    TRestGeant4EventExtended(const G4Event*);
};

@jgalan
Copy link
Member

jgalan commented Jul 5, 2021

Ok, then that does not imply changes on geant4lib?

@lobis
Copy link
Member Author

lobis commented Jul 5, 2021

Ok, then that does not imply changes on geant4lib?

In principle no, only that some methods would be deprecated once this is implemented (we could keep them for compatibility reasons).

But I still think its relevant to put this issue here, since it will take functionality away from geant4lib

@jgalan
Copy link
Member

jgalan commented Jul 5, 2021

Ok, then any new package implemented would need to integrate its own interface? We could probably leave the AddG4Hit in order to allow other future simulation packages to produce a Geant4 event without additional coding.

@lobis
Copy link
Member Author

lobis commented Jul 5, 2021

Ok, then any new package implemented would need to integrate its own interface? We could probably leave the AddG4Hit in order to allow other future simulation packages to produce a Geant4 event without additional coding.

Agree, altought in my opinion geant4lib classes need a lot of decluttering. I will begin implementing the corresponding changes also to https://github.com/rest-for-physics/restGeant4 to see how it works.

@lobis
Copy link
Member Author

lobis commented Sep 28, 2022

Implemented in #59

@lobis lobis closed this as completed Sep 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants