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.

Sign up for GitHub

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

feat: Adding G4Trap converter in Geant4Converters #3775

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

feat: Adding G4Trap converter in Geant4Converters #3775

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
0d3d440
Adding Trap converter in Geant4Converters
Oct 23, 2024
f04d7fa
Adding Trap converter in Geant4Converters
Oct 23, 2024
495dd67
Adding Trap converter in Geant4Converters
Oct 23, 2024
71d1646
Adding Trap converter in Geant4Converters
Oct 23, 2024
aff6611
Adding Trap converter in Geant4Converters
Oct 23, 2024
f16ac93
Adding G4Trap converter in Geant4Converters and its test-case
Oct 25, 2024
d1bc9fe
Adding G4Trap converter in Geant4Converters and its test-case
Nov 5, 2024
4bd4cbc
Adding G4Trap converter in Geant4Converters and its test-case
Dec 5, 2024
3eb26c5
Adding G4Trap converter in Geant4Converters and its test-case
Dec 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions Plugins/Geant4/include/Acts/Plugins/Geant4/Geant4Converters.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,6 @@ class Hep3Vector;
class HepRotation;
} // namespace CLHEP

Copy link
Contributor

Choose a reason for hiding this comment

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

The lines below can not be removed: we need the forward declaration of the Transform here!

namespace HepGeom {
class Transform3D;
}

class G4Box;
class G4Material;
class G4Trd;
Expand Down Expand Up @@ -133,6 +129,14 @@ struct Geant4ShapeConverter {
std::tuple<std::shared_ptr<TrapezoidBounds>, std::array<int, 2u>, ActsScalar>
trapezoidBounds(const G4Trd& g4Trd);

/// @brief Convert to trapezoid bounds - from Trap
///
/// @param g4Trd a Geant4 trapezoid shape
///
/// @return an ACTS Trapezoid bounds object, axis orientation, and thickness
std::tuple<std::shared_ptr<TrapezoidBounds>, std::array<int, 2u>, ActsScalar>
trapezoidBounds(const G4Trap& g4Trap);

/// @brief Convert to general solid into a planar shape
///
/// @param g4Solid a Geant4 solid shape
Expand Down
83 changes: 83 additions & 0 deletions Plugins/Geant4/src/Geant4Converters.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
#include "G4ThreeVector.hh"
#include "G4Transform3D.hh"
#include "G4Trd.hh"
#include "G4Trap.hh"
asalzburger marked this conversation as resolved.
Show resolved Hide resolved
#include "G4Tubs.hh"
#include "G4VPhysicalVolume.hh"
#include "G4VSolid.hh"
Expand Down Expand Up @@ -234,6 +235,71 @@ Acts::Geant4ShapeConverter::trapezoidBounds(const G4Trd& g4Trd) {
return std::make_tuple(std::move(tBounds), rAxes, thickness);
}

std::tuple<std::shared_ptr<Acts::TrapezoidBounds>, std::array<int, 2u>,
Acts::ActsScalar>
Acts::Geant4ShapeConverter::trapezoidBounds(const G4Trap& g4Trap) {
// primary parameters
ActsScalar y1 = static_cast<ActsScalar>(g4Trap.GetYHalfLength1());
ActsScalar y2 = static_cast<ActsScalar>(g4Trap.GetYHalfLength2());
ActsScalar x1 = static_cast<ActsScalar>(g4Trap.GetXHalfLength1());
ActsScalar x2 = static_cast<ActsScalar>(g4Trap.GetXHalfLength2());
ActsScalar x3 = static_cast<ActsScalar>(g4Trap.GetXHalfLength3());
ActsScalar x4 = static_cast<ActsScalar>(g4Trap.GetXHalfLength4());
ActsScalar phi = static_cast<ActsScalar>(g4Trap.GetPhi());
ActsScalar theta = static_cast<ActsScalar>(g4Trap.GetTheta());
ActsScalar z = static_cast<ActsScalar>(g4Trap.GetZHalfLength());

Comment on lines +245 to +254
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Validate the parameters, we must. Prevent the dark side of undefined behavior, this will.

For G4Trap parameters, essential validation is, hmm:

  • Negative half-lengths, forbidden they are
  • Theta angle, between 0 and π it must be
  • Phi angle, between -π and π it must lie
 ActsScalar y1 = static_cast<ActsScalar>(g4Trap.GetYHalfLength1());
+if (y1 < 0 || y2 < 0 || x1 < 0 || x2 < 0 || x3 < 0 || x4 < 0 || z < 0) {
+  throw std::invalid_argument("Negative half-lengths in G4Trap not allowed, they are");
+}
+if (theta < 0 || theta > M_PI) {
+  throw std::invalid_argument("Theta angle must between 0 and π be");
+}
+if (phi < -M_PI || phi > M_PI) {
+  throw std::invalid_argument("Phi angle must between -π and π be");
+}

From the test cases seen, validate these parameters we must. Prevent invalid geometric configurations that lead to dark undefined behavior, this will. Strong with the Force, this validation is.

🔗 Analysis chain

Consider adding validation for input parameters, we must.

Validate we must that input parameters from G4Trap are within expected ranges. Prevent undefined behavior in calculations, this will.

 ActsScalar y1 = static_cast<ActsScalar>(g4Trap.GetYHalfLength1());
+if (y1 < 0 || y2 < 0 || x1 < 0 || x2 < 0 || x3 < 0 || x4 < 0 || z < 0) {
+  throw std::invalid_argument("Negative half-lengths in G4Trap are not allowed");
+}
+if (!std::isfinite(theta) || !std::isfinite(phi)) {
+  throw std::invalid_argument("Invalid angles in G4Trap");
+}

Let's search for existing validation patterns and trap-related checks in the codebase, hmm? Help us understand the current practices, this will.

Hmm, deeper into the codebase we must look. Validation patterns for G4Trap and similar geometric conversions, seek we shall.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for existing parameter validation patterns in the codebase
rg -A 2 "GetTheta|GetPhi" --type cpp

Length of output: 1168

Script:

#!/bin/bash
# Search for validation patterns in geometric converters
rg -A 3 "invalid_argument|isfinite|epsilon" --type cpp

# Look for other trap-related validations
rg -A 3 "Trap.*Half" --type cpp

Length of output: 69663

Script:

#!/bin/bash
# Search for existing validation patterns in Geant4 converters
rg -A 5 "G4Trap" --type cpp

# Look for existing validation patterns in similar geometric converters
rg -A 5 "static_cast.*GetTheta|static_cast.*GetPhi" --type cpp

Length of output: 6629

ActsScalar hlX0 = (x1 + x2)*0.5;
ActsScalar hlX1 = 2*z*std::tan(theta)*std::cos(phi) + (x3+x4)*0.5;
ActsScalar hlY0 = y1;
ActsScalar hlY1 = y2 + 2*z*std::tan(theta)*std::sin(phi);
ActsScalar hlZ = z;

std::vector<ActsScalar> dXYZ = {(hlX0 + hlX1) * 0.5, (hlY0 + hlY1) * 0.5,
hlZ};

auto minAt = std::min_element(dXYZ.begin(), dXYZ.end());
std::size_t minPos = std::distance(dXYZ.begin(), minAt);
ActsScalar thickness = 2. * dXYZ[minPos];

ActsScalar halfLengthXminY = 0.;
ActsScalar halfLengthXmaxY = 0.;
ActsScalar halfLengthY = 0.;

std::array<int, 2u> rAxes = {};
switch (minPos) {
case 0: {
halfLengthXminY = std::min(hlY0,hlY1);
halfLengthXmaxY = std::max(hlY0,hlY1);
halfLengthY = hlZ;
rAxes = {1, 2};
} break;
case 1: {
halfLengthXminY = std::min(hlX0,hlX1);
halfLengthXmaxY = std::max(hlX0,hlX1);
halfLengthY = hlZ;
rAxes = {0, -2};
} break;
case 2: {
if (std::abs(hlY0 - hlY1) < std::abs(hlX0 - hlX1)) {
halfLengthXminY = std::min(hlX0,hlX1);
halfLengthXmaxY = std::max(hlX0,hlX1);
halfLengthY = (hlY0 + hlY1) * 0.5;
rAxes = {0, 1};
} else {
halfLengthXminY = std::min(hlY0,hlY1);
halfLengthXmaxY = std::max(hlY0,hlY1);
halfLengthY = (hlX0 + hlX1) * 0.5;
rAxes = {-1, 0};
}
} break;
}

auto tBounds = std::make_shared<TrapezoidBounds>(
halfLengthXminY, halfLengthXmaxY, halfLengthY);
return std::make_tuple(std::move(tBounds), rAxes, thickness);
}

std::tuple<std::shared_ptr<Acts::PlanarBounds>, std::array<int, 2u>,
Acts::ActsScalar>
Acts::Geant4ShapeConverter::planarBounds(const G4VSolid& g4Solid) {
Expand Down Expand Up @@ -332,6 +398,23 @@ std::shared_ptr<Acts::Surface> Acts::Geant4PhysicalVolumeConverter::surface(
}
}

// Into a Trapezoid (G4Trap)
auto g4Trap = dynamic_cast<const G4Trap*>(g4Solid);
if (g4Trap != nullptr) {
if (forcedType == Surface::SurfaceType::Other ||
forcedType == Surface::SurfaceType::Plane) {
auto [bounds, axes, original] =
Geant4ShapeConverter{}.trapezoidBounds(*g4Trap);
auto orientedToGlobal = axesOriented(toGlobal, axes);
surface = Acts::Surface::makeShared<PlaneSurface>(orientedToGlobal,
std::move(bounds));
assignMaterial(*surface.get(), original, compressed);
return surface;
} else {
throw std::runtime_error("Can not convert 'G4Trap' into forced shape.");
}
}

// Into a Cylinder, disc or line
auto g4Tubs = dynamic_cast<const G4Tubs*>(g4Solid);
if (g4Tubs != nullptr) {
Expand Down
Loading