Refactoring the new Async backend modifications #318
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Refactor the processing of hits. Instead of processing hits by passing a pointer/reference to a HostScoring instance, a loop over iterators to hits is used. In this way, hit scoring is decoupled from the specific implementation of HostScoring, and all classes with the same interface as the original GPUHit can be used for scoring. This facilitates hit scoring for the AsyncTransport implementation. - Move several Geant4 objects into the .cpp to make the integration headers simpler. - Place temporary scoring objects into a struct to go around G4's pool allocators. This prevents a destruction order fiasco (where the pool is gone but the object isn't), and keeps the scoring objects closer in memory. A few objects need to leak, unfortunately, since they are allocated in G4 pools, and the handles don't support them being on a stack. - Improve const correctness in a few places. - Add information about threadID and eventID to the scoring interface. This information is required for AsyncAdePT to score correctly, but is unused in the thread-local transport for now.
- Split the transport from the integration-related parts introducing a new source file. The integration-related parts can be reused in a different transport implementation. - Create a transport abstraction, so AdePTTrackingManager is independent of the transport implementation. - Add thread and event IDs to the transport interface. These are necessary for the async transport implementation. - Start to enumerate tracks in the tracking manager. This can be used to reproducibly seed the AdePT random sequences. - Add some const declarations for the default AdePT implementation. - Use a factory function to instantiate AdePT. Like this, different AdePT implementations can be used without changing code in the tracking manager or in AdePTPhysics. - Replace a few includes with forward declarations. - Fix device link errors that can show when using a symbol in multiple cuda translation units.
|
Can one of the admins verify this patch? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The goal of this PR is to adapt the new Asynchronous AdePT components to the way the library is structured. This involves keeping a clear separation between AdePT and the integration layer.
Main changes:
Remove
AdePTConfigurationfrom the integration layer.This is an utility class which holds the AdePT configuration for a run, it is part of AdePT and independent from the integration layer. However the new
AdePTConfiguration::CreateAdePTInstanceis compiled as part ofAdePTG4Integration. In practice, this isn't a change from the previous implementation, where the integration layer has the responsability of initializingAdePTConfiguration, and also of passing the values inAdePTConfigurationtoAdePTTransport.This will be changed in this PR, by introducing a new
AdePTTransportconstructor which takes anAdePTConfigurationinstance as an argument. Now the integration layer is only responsible of initializingAdePTConfiguration.AdePTConfigurationwill now also store the number of threads, which is used to compute the number of tracks and step slots allocated to each thread in synchronous mode. By doing this, we will also remove the need to store the number of slots per thread asstatic inlinevariables inAdePTTransport. This was done because in Geant4 we can only get the correct number of threads from the master, as the workers will report 1, thus we can only compute the space per thread from the master. However this is a G4-specific requirement that is not related to AdePT, so now, instead of doing that, we store the number of threads as astatic inlinevariable inAdePTTrackingManager, on the integration side, which each thread then passes on toAdePTConfiguration.Remove the
AdePTTransportFactorymethod in favour of using theAdePTTransportconstructor.This method is currently implemented as a free function that is part of the integration layer. It is called from
AdePTConfiguration::CreateAdePTInstance.In principle, it wouldn't be an issue to keep one factory per transport backed, per integration layer, however at least for the synchronous backend there seems to be little reason to use a factory method instead the
AdePTTransportconstructor.In order to
Implement a constructor in
AdePTTransportwhich takes anAdePTConfigurationinstance as an argument.In the current implementation, the integration layer is supposed to initialize
AdePTConfigurationand also to pass those values on toAdePTTransport. With this change, the integration layer only needs to initialize the configuration.Move the instantiation of
ShowerGPUback to toAdePTTrackingManager.cu.We need to instantiate
ShowerGPUin a.cufile so that this function is compiled bynvcc. This is a necessary evil, and needs to be done in the integration layer since AdePT is a header-only library. This function has currently been moved toAdePTTransport.cu, which lives on the integration layer. As with theAdePTConfigurationchange, in practice the integration layer is still responsible of doing this instantiation, it just lives on a different file. SinceAdePTTransport.cuis not actually related toAdePTTransport, and it isAdePTTrackingManagerthat calls this function, I believe it is less misleading to keep this inAdePTTrackingManager.cu.