test: geant4 python example

[andiwand]

similar to fatras.py I tried to add a Geant4 utility function to add it to the chain instead of fatras

[andiwand]

any thoughts on this @paulgessinger @timadye ?

we could also run this in the CI or use it to run Geant4 in the CI

[codecov]

Codecov Report

Merging #1334 (997245a) into main (6fe9ca2) will not change coverage.
The diff coverage is n/a.

❗ Current head 997245a differs from pull request most recent head a78a045. Consider uploading reports for the commit a78a045 to get more accurate results

@@           Coverage Diff           @@
##             main    #1334   +/-   ##
=======================================
  Coverage   47.46%   47.46%           
=======================================
  Files         375      375           
  Lines       19827    19827           
  Branches     9297     9297           
=======================================
  Hits         9410     9410           
  Misses       4033     4033           
  Partials     6384     6384           

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

[paulgessinger]

This is a very good idea, thanks!

[timadye]

Looks good. Just a couple of suggestions:

• Can this be coordinated with refactor: Move add* functions to python modules #1309? Probably simplest is to make a similar move as done there, moving addGeant4 to the new Examples/Python/python/acts/examples/simulation.py, but keep runGeant4 where it is.
• Comparing with addFatras, I notice that addGeant4 doesn't have any output writers. I guess they would work the same as for Fatras (since both output to particles_final etc), so the same code could be copied. That's potentially a lot of duplicated code, so maybe could be done with an internal routine (addFratrasWriters?) called by both addGeant4 and addFatras in acts.examples.simulation.

What do you think?

[andiwand]

those are very good suggestions @timadye thank you! I am going to implement this next week

[andiwand]

I implemented your suggestions @timadye . do you know when #1309 will be ready? I guess we can merge #1309 or #1334 first either way and deal with the conflicts in the remaining PR. happy to do it in #1309 too

[timadye]

do you know when #1309 will be ready? I guess we can merge #1309 or #1334 first either way and deal with the conflicts in the remaining PR. happy to do it in #1309 too

Hopefully @tboldagh and @paulgessinger can comment on the best way/order to do this.

[tboldagh]

do you know when #1309 will be ready?
I actively look into is now. Struggling a bit with ACTS complex build system though. But earlier today I was able to reproduce the issue seen online. So far so good.

[tboldagh]

Need an advice one one thing here. The getOpenDataDetecotrDirectory discovery mechanism for the ODD files path relied on the fact the the __file__ was pointing in fact to the sources directory. However, now when above function is installed the __file__ points to the build dir. I though that the way to go is to use DD4hep_DIR env variable. But I am struggling with it actually. @timadye @paulgessinger

[paulgessinger]

That's a good point. I think these functions can't really work in the installed case, since they assume they know where ODD is located.
I don't think we can assume this in the installed case.

Do we need to support this autoconfig ODD mode if ACTS is installed?

[andiwand]

in this case we might want to install the ODD as well as part of Acts. but we might overshoot the original goal

[timadye]

Alternatively, could we treat the ODD like we treat the ITk and let the user specify the base directory? For the ITk, we need to do that because it's in a separate repo, but we could use a similar mechanism for the ODD.

[paulgessinger]

We use the ODD all over the place in the python based tests and it's our performance baseline. I think it's reasonable to have ODD work "out-of-the-box" in the most common scenario.

[tboldagh]

Maybe odd.py could only contain the getOpenDataDetector whereas the getOpenDataDetectorDirectory could go back to the sources dir and thus allow the discovery. Archeologists will wonder though.

[timadye]

Maybe odd.py could only contain the getOpenDataDetector whereas the getOpenDataDetectorDirectory could go back to the sources dir and thus allow the discovery. Archeologists will wonder though.

How do we refer to the source directory in the import? If we kept the current layout, it would mean ODD scripts would only work if run from the source directory, which was one of the things this change was supposed to fix.

[andiwand]

Should we actually run this from pytest @andiwand? Might have to go through subprocess again because G4.

good idea will do

[andiwand]

this should be ready now

[paulgessinger]

Sorry for more comments. 😅

[andiwand]

I think I addressed all your comments @paulgessinger