Skip to content

Commit

Permalink
fix: Gap translations in Blueprint Helper (#3027)
Browse files Browse the repository at this point in the history
Introducing small changes into the way translations are handled in the `BlueprintHelper` gap filling. 

Previously, because the rotation of a gap volume was applied before the translation, the gaps were positioned in the wrong points of the global coordinates. This was due to the fact that `negC` and `posC` are already defined in the rotated frame. The change solidifies the global way of defining the transformations in the user code, which seems to be consistent with the way it is handled across the `Blueprint`. 
 
To give an example: if the user code tries to fill the gaps in the detector rotated by `pi/2`, the `negC`, `posC` would've been calculated along the already rotated axis. Then, the gap transformation would've been constructed and the filling volume would've been sent along the `pi` with respect to the initial global coordinates, as the rotation would've been effectively applied twice. Now, the fact that the rotation is already present in the translation vector is taken into account.
  • Loading branch information
ssdetlab authored Mar 18, 2024
1 parent 44dd2ab commit 93b4383
Showing 1 changed file with 4 additions and 8 deletions.
12 changes: 4 additions & 8 deletions Core/src/Detector/detail/BlueprintHelper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ void Acts::Experimental::detail::BlueprintHelper::fillGapsCylindrical(
auto gapName = node.name + "_gap_" + std::to_string(igap);
auto gapTransform = Transform3::Identity();
gapTransform.rotate(node.transform.rotation());
gapTransform.translate(0.5 * (neg + negC));
gapTransform.pretranslate(0.5 * (neg + negC));
auto gap = std::make_unique<Blueprint::Node>(
gapName, gapTransform, VolumeBounds::eCylinder,
std::vector<ActsScalar>{cInnerR, cOuterR, 0.5 * gapSpan});
Expand All @@ -148,7 +148,7 @@ void Acts::Experimental::detail::BlueprintHelper::fillGapsCylindrical(
auto gapName = node.name + "_gap_" + std::to_string(igap);
auto gapTransform = Transform3::Identity();
gapTransform.rotate(node.transform.rotation());
gapTransform.translate(0.5 * (negC + posC));
gapTransform.pretranslate(0.5 * (negC + posC));
auto gap = std::make_unique<Blueprint::Node>(
gapName, gapTransform, VolumeBounds::eCylinder,
std::vector<ActsScalar>{cInnerR, cOuterR, 0.5 * gapSpan});
Expand Down Expand Up @@ -237,17 +237,13 @@ void Acts::Experimental::detail::BlueprintHelper::fillGapsCuboidal(
unsigned int igap = 0;
for (auto& child : node.children) {
auto [neg, pos] = endPointsXYZ(*child, binVal);
if (neg[binVal] < negC[binVal]) {
throw std::runtime_error("BlueprintHelper: Overlap detected for child '" +
child->name + "' of node '" + node.name + "'");
}
ActsScalar gapSpan = (neg - negC).norm();
if (gapSpan > s_onSurfaceTolerance) {
// Fill a gap node
auto gapName = node.name + "_gap_" + std::to_string(igap);
auto gapTransform = Transform3::Identity();
gapTransform.rotate(node.transform.rotation());
gapTransform.translate(0.5 * (neg + negC));
gapTransform.pretranslate(0.5 * (neg + negC));
std::vector<ActsScalar> gapBounds{0, 0, 0};
gapBounds[binVal] = 0.5 * gapSpan;
for (auto bv : allowedBinVals) {
Expand All @@ -270,7 +266,7 @@ void Acts::Experimental::detail::BlueprintHelper::fillGapsCuboidal(
auto gapName = node.name + "_gap_" + std::to_string(igap);
auto gapTransform = Transform3::Identity();
gapTransform.rotate(node.transform.rotation());
gapTransform.translate(0.5 * (negC + posC));
gapTransform.pretranslate(0.5 * (negC + posC));
std::vector<ActsScalar> gapBounds{0, 0, 0};
gapBounds[binVal] = 0.5 * gapSpan;
for (auto bv : allowedBinVals) {
Expand Down

0 comments on commit 93b4383

Please sign in to comment.