search

celeritas-project / celeritas

Celeritas is a new Monte Carlo transport code designed to accelerate scientific discovery in high energy physics by improving detector simulation throughput and energy efficiency using GPUs.
https://celeritas-project.github.io/celeritas/user/index.html
Other
62 stars 34 forks source link

Import scintillation particle data #1151

Closed stognini closed 7 months ago

stognini commented 7 months ago

This PR imports scintillation data for multiple particle types from Geant4 as a part of #886 .

Important caveat: ScintillationParams and ScintillationData are not updated to correctly use the imported data, this will be done in a follow-up PR.

stognini commented 7 months ago

Interesting... the CI errors are all the same

ROOT: error: TList::Clear: A list is accessing an object (0xN) already deleted (list name = TList)

But the tests pass on my local build. I'll try to figure this out.

pcanal commented 7 months ago

Which version of ROOT is being used for the failing run? i.e. are we hitting https://github.com/root-project/root/issues/14793 ?

stognini commented 7 months ago

Could you add the new properties to test/geocel/data/lar-sphere.gdml and test that they are imported properly in the GeantImporter test

Good point! Will do.

stognini commented 7 months ago

Which version of ROOT is being used for the failing run? i.e. are we hitting https://github.com/root-project/root/issues/14793 ?

Oh, that may be it. My local ROOT is 6.31/01, which might be newer than the CI's one.

stognini commented 7 months ago

Thanks @sethrj , now working on @amandalund 's request for the GeantImporter test.

whokion commented 7 months ago

@stognini @sethrj sorry for delayed the review. Please allow me one more day for reviewing this MR. Thanks.

sethrj commented 7 months ago

Of course! Take your time, it's hardly a day old.

stognini commented 7 months ago

Okay @amandalund , I fixed the missing insert() you found (again, great catch, thanks), added particle data to the lar-sphere.gdml, and expanded the GeantImporter test.

sethrj commented 7 months ago

@stognini It looks like the only remaining issue is with the ROOT compatibility. I think either of us could use the build on wildstyle to replace the ROOT data with a backward compatible version?

Also, can you resolve all the conversations that you've addressed? Thanks!

stognini commented 7 months ago

I think either of us could use the build on wildstyle to replace the ROOT data with a backward compatible version

Oh, good point. I was building an older ROOT locally, but using wildstyle is a good idea. On it.