Skip to content

Add support for displaced solid#34

Merged
sethrj merged 6 commits intoceleritas-project:mainfrom
sethrj:displaced-solid
Jun 30, 2025
Merged

Add support for displaced solid#34
sethrj merged 6 commits intoceleritas-project:mainfrom
sethrj:displaced-solid

Conversation

@sethrj
Copy link
Member

@sethrj sethrj commented Jun 17, 2025

This models a displaced solid as an intersection of a regular solid with an infinite sphere. It'll obviously be less efficient than directly transforming a solid but it's at least a way of giving the capability now and waiting for optimization later.

@sethrj sethrj requested a review from agheata June 17, 2025 01:23
Copy link
Collaborator

@agheata agheata left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest to use a box instead an orb, other than that I think this is good to be merged

@sethrj
Copy link
Member Author

sethrj commented Jun 17, 2025

Doesn't a box have more data and use more conditionals to test "is inside"?

Also can you please provide an example of the failing geometry so I can test? 😊

@agheata
Copy link
Collaborator

agheata commented Jun 17, 2025

Doesn't a box have more data and use more conditionals to test "is inside"?

Yes but what matters most is the distance computation

Also can you please provide an example of the failing geometry so I can test? 😊

We only found it in he full ATLAS, and we cannot have a GDML version of that, because GDML does not support displaced solids. To test, you can modify a line in whatever G4 example, e.g. at https://gitlab.cern.ch/geant4/geant4-dev/-/blob/master/examples/extended/persistency/gdml/G02/src/G02DetectorConstruction.cc#L460
you can replace the line with:

  G4Tubs* BigTube_initial = new G4Tubs("BTube", 0, smalL, smalL, -pi / 2., pi);
  G4ReflectedSolid* BigTube = new G4ReflectedSolid("BTube_moved", BigTube_initial, G4Transform3D::Identity);

and run G4VG on that geometry, but you can do it with even simpler examples by replacing any of the solids with G4ReflectedSolid of that solid, providing an identity transformation.

@sethrj
Copy link
Member Author

sethrj commented Jun 18, 2025

I'm confused about the example: is that a reflection of a reflection, with an empty transform occupying the original space? :\ I've just added a test with a raw displaced solid. I also am using a union with a zero-size unplaced box, since the intersection capacity returned nan.

@sethrj sethrj marked this pull request as ready for review June 18, 2025 13:50
@agheata
Copy link
Collaborator

agheata commented Jun 18, 2025

I'm confused about the example: is that a reflection of a reflection, with an empty transform occupying the original space? :\ I've just added a test with a raw displaced solid. I also am using a union with a zero-size unplaced box, since the intersection capacity returned nan.

I'm not sure what the example does, but it does not matter as long as it gives the same output when replacing with a zero-displaced solid. Probably the example replacing a reflection is not the best, indeed. The capacity can only be estimated by sampling, and this gives nan because there are no points sampled inside due to the very big box. Union is problematic because it generates a bounding box that contains the origin, even if the displaced solid is located far away. You can still use the intersection, but you don't need to use an infinity box; just use one with a size equal to the magnitude of the displaced solid translation components plus the diagonal of the undisplaced solid extent.

@sethrj
Copy link
Member Author

sethrj commented Jun 18, 2025

it does not matter as long as it gives the same output when replacing with a zero-displaced solid

We can't really test whether our implementation is correct unless the displaced and non-displaced solids are distinguishable.

But anyway, I think the implementation is complete and working: give it a try!

@sethrj
Copy link
Member Author

sethrj commented Jun 20, 2025

@agheata @SeverinDiederichs @drbenmorgan If you can test this with ATLAS, please let me know if it works!

@SeverinDiederichs
Copy link
Collaborator

Thanks for this important addition @sethrj, greatly appreciated!
I can confirm that with this PR the full ATLAS geometry can be converted and runs (and without the PR, obviously it fails due to the missing displaced solid). I cannot say anything about the correctness at this point, but that doesn't matter. We will not mark the regions with the displaced solids for GPU tracking for now, so we only need to pass the construction phase, so this PR fully serves that purpose.

@sethrj sethrj requested a review from SeverinDiederichs June 29, 2025 12:07
Copy link
Collaborator

@SeverinDiederichs SeverinDiederichs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the addition!

@sethrj sethrj added the enhancement New feature or request label Jun 30, 2025
@sethrj sethrj merged commit 07031ad into celeritas-project:main Jun 30, 2025
3 checks passed
@sethrj sethrj deleted the displaced-solid branch June 30, 2025 08:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants