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

Expand vertex file reader to parse particle type and momenta #156

Open
gipert opened this issue Nov 10, 2024 · 16 comments · May be fixed by #239
Open

Expand vertex file reader to parse particle type and momenta #156

gipert opened this issue Nov 10, 2024 · 16 comments · May be fixed by #239
Assignees
Labels
enhancement New feature or request generators Event generators

Comments

@gipert
Copy link
Member

gipert commented Nov 10, 2024

This would make remage support a wide range of external event generators through RMGVertexFromFile. The implementation would need to be a bit more complex than what it is right now, since the input file structure is "jagged". I guess a lot of this is already implemented in RMGGrabmayrGCReader.

Actually, "vertex" in RMGVertexFromFile was originally intended to refer only to vertex position. We should maybe implement all of this as an additional generator.

@gipert gipert added enhancement New feature or request generators Event generators labels Nov 10, 2024
@tdixon97
Copy link
Collaborator

tdixon97 commented Nov 11, 2024

I dont think the table necessarily needs to be jagged, we just need to include the evtid (and time?).

I think the option to read kinematics as a table like:

xpos,ypos,zpos, px,py,pz, particle, time, evtid (all as Arrays)

would be useful and only a relatively small modification of the current code. It might be nice to read LH5 not h5 files however.

@gipert
Copy link
Member Author

gipert commented Nov 11, 2024

Yes, I meant that the number of particles originating at the vertex is not fixed.

@tdixon97
Copy link
Collaborator

Started prototyping a vertex generator here: pyvertexgen.
It should be easy to write to a HDF5 file and start testing this.

@ManuelHu
Copy link
Collaborator

yes, but you will need to write it in geant4's ugly HDF5 ntuple format (are there even docs for it?). An LH5 reader would be very hard to add to the geant4 "analysis framework".

@tdixon97
Copy link
Collaborator

I don't know much about it, maybe we can write a simple converter like is done for the output?

@tdixon97
Copy link
Collaborator

I had a look and I am not sure how to create the Geant4 Ntuple inputs. Maybe we should go for something like the RMGGrabmayrGCReader as @gipert suggested and forget the geant4 input format.

@gipert
Copy link
Member Author

gipert commented Dec 2, 2024

  • we want to use LH5
  • no support for positions and momenta in the same table
  • positions is simple flat table (x, y, z)
  • momenta is still a flat table. Columns: evtid, g4_pid, px, py, pz

@tdixon97
Copy link
Collaborator

@ManuelHu what is the status of this? I recall you had something working

@ManuelHu
Copy link
Collaborator

not really. I have the general infrastructure to read LH5 files as input in a multithreaded run; but I did not yet work on the particle creation side. Only the vertex (position) reader works at the moment.

And I also still need to push some code I worked on locally, to really decouple the LH5 reading part from the vertex reader. After that, the rest should be rather easy.

@tdixon97
Copy link
Collaborator

I might be able to find some time to look into creating the vertices if the LH5 reader works. I think it should be fairly straightforward and I'd like to do this relatively soon.

@ManuelHu
Copy link
Collaborator

ManuelHu commented Jan 19, 2025

@tdixon: see #230 for the refactor. The RMGAnalysisReader can only be initialized from the main thread. Apart from that, it should be quite simple to use and adapt.

note: there is no sane way to access/check the structure of the input file with the G4*AnalysisReaders. The structure has always to be the fixed (columns might be optional though).

@tdixon97
Copy link
Collaborator

tdixon97 commented Jan 19, 2025

This looks great @ManuelHu , maybe you could add some docstrings to the main functions ? That would make it easier for people to use this reader for particular applications, and would be a step towards having proper documentation.
Can this read any LH5 file or just the geant4 ntuple format? We should definitely document what format the input file needs to be in.

@tdixon97
Copy link
Collaborator

I think we should also add some test on multithreading / thread safety since that makes me nervous..
We could generate some LH5 file of positions, read it in in multithread mode and then compare the output vertex table to the input ?

@ManuelHu
Copy link
Collaborator

ManuelHu commented Jan 21, 2025

Sure. I am also worried quite a lot, the whole situation is very bad:

  • the analysis readers automatically mangle names when used on worker threads.
    • but we cannot pre-compute input files; as we don't know the number of rows needed
    • so we can only use them on the main thread
  • analysis readers are thread-local by file type. there is no way to create a new reader that is not connected to this global instance
  • So if we read two HDF5 files in different parts of remage that will be racy, unless we use a global lock around any analysisreader usage.
  • ...and we just need to hope that Geant4 does not use it internally
  • ouch

@tdixon97
Copy link
Collaborator

tdixon97 commented Jan 21, 2025 via email

@ManuelHu
Copy link
Collaborator

I imagine an (ugly) solution. Maybe it works, but I would really test before that if we can actually concurrently read from two files before investing time into making this safe and simple to use.
If it does not work in that simple case (reading vtx and kin data), we would need to scrap the whole solution and implement the input reading by hand :-/

@ManuelHu ManuelHu linked a pull request Jan 23, 2025 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request generators Event generators
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants