Skip to content

Commit

Permalink
fix: CylVolStack resizing issue (#3715)
Browse files Browse the repository at this point in the history
This fixes a bug where the local transform was not correctly synced
after resizing. Also improves a number of assertions to not rely on
floating point identity anymore

Related to #3502

---

This pull request includes several changes to the `CylinderVolumeStack` and `Volume` classes to improve logging, add new parameters, and enhance the overlap checking and volume updating mechanisms. The most important changes include adding a logger parameter to several methods, updating method signatures, and improving overlap checking logic.

### Enhancements to logging and method signatures:

* [`Core/include/Acts/Geometry/CylinderVolumeStack.hpp`](diffhunk://#diff-86daf525fefbaa344566ea4fc6493ca85afd3627922f04c3f16758fca8717a6eL88-R93): Added a `logger` parameter to the `update` method and modified its signature to include this parameter. [[1]](diffhunk://#diff-86daf525fefbaa344566ea4fc6493ca85afd3627922f04c3f16758fca8717a6eL88-R93) [[2]](diffhunk://#diff-86daf525fefbaa344566ea4fc6493ca85afd3627922f04c3f16758fca8717a6eR125-R130)
* [`Core/include/Acts/Geometry/Volume.hpp`](diffhunk://#diff-beb0194dfdd77fcfe13c0b73c9e7790cbe7bae313f1723f7979a4e9beb5d16d5R16): Added a `logger` parameter to the `update` method and included the necessary import for the `Logger` class. [[1]](diffhunk://#diff-beb0194dfdd77fcfe13c0b73c9e7790cbe7bae313f1723f7979a4e9beb5d16d5R16) [[2]](diffhunk://#diff-beb0194dfdd77fcfe13c0b73c9e7790cbe7bae313f1723f7979a4e9beb5d16d5R87-R90)
* [`Core/src/Geometry/CylinderVolumeStack.cpp`](diffhunk://#diff-39babeb41776345f10ffd90e329b67faf7a8a4f47f0abe7533f665442d52a44cL70-R70): Updated the `commit` method to include a `logger` parameter and modified the `initializeOuterVolume` method to pass the `logger` parameter to the `update` method. [[1]](diffhunk://#diff-39babeb41776345f10ffd90e329b67faf7a8a4f47f0abe7533f665442d52a44cL70-R70) [[2]](diffhunk://#diff-39babeb41776345f10ffd90e329b67faf7a8a4f47f0abe7533f665442d52a44cL154-R156) [[3]](diffhunk://#diff-39babeb41776345f10ffd90e329b67faf7a8a4f47f0abe7533f665442d52a44cL188-R190) [[4]](diffhunk://#diff-39babeb41776345f10ffd90e329b67faf7a8a4f47f0abe7533f665442d52a44cL212-R215) [[5]](diffhunk://#diff-39babeb41776345f10ffd90e329b67faf7a8a4f47f0abe7533f665442d52a44cL244-R247) [[6]](diffhunk://#diff-39babeb41776345f10ffd90e329b67faf7a8a4f47f0abe7533f665442d52a44cL268-R272)

### Improvements to overlap checking:

* [`Core/src/Geometry/CylinderVolumeStack.cpp`](diffhunk://#diff-39babeb41776345f10ffd90e329b67faf7a8a4f47f0abe7533f665442d52a44cL286-R301): Enhanced the `overlapPrint` method to include a `direction` parameter and updated the overlap checking logic to handle different directions more robustly. [[1]](diffhunk://#diff-39babeb41776345f10ffd90e329b67faf7a8a4f47f0abe7533f665442d52a44cL286-R301) [[2]](diffhunk://#diff-39babeb41776345f10ffd90e329b67faf7a8a4f47f0abe7533f665442d52a44cR312-R323) [[3]](diffhunk://#diff-39babeb41776345f10ffd90e329b67faf7a8a4f47f0abe7533f665442d52a44cL319-R338) [[4]](diffhunk://#diff-39babeb41776345f10ffd90e329b67faf7a8a4f47f0abe7533f665442d52a44cL347-R368) [[5]](diffhunk://#diff-39babeb41776345f10ffd90e329b67faf7a8a4f47f0abe7533f665442d52a44cL357-R386) [[6]](diffhunk://#diff-39babeb41776345f10ffd90e329b67faf7a8a4f47f0abe7533f665442d52a44cL379-R405) [[7]](diffhunk://#diff-39babeb41776345f10ffd90e329b67faf7a8a4f47f0abe7533f665442d52a44cL397-R424) [[8]](diffhunk://#diff-39babeb41776345f10ffd90e329b67faf7a8a4f47f0abe7533f665442d52a44cR436-R442) [[9]](diffhunk://#diff-39babeb41776345f10ffd90e329b67faf7a8a4f47f0abe7533f665442d52a44cL446-R472)

### Volume update improvements:

* [`Core/src/Geometry/CylinderVolumeStack.cpp`](diffhunk://#diff-39babeb41776345f10ffd90e329b67faf7a8a4f47f0abe7533f665442d52a44cL625-R689): Updated the `update` method to provide detailed logging of the current and new bounds, and added checks to prevent shrinking the stack size. [[1]](diffhunk://#diff-39babeb41776345f10ffd90e329b67faf7a8a4f47f0abe7533f665442d52a44cL625-R689) [[2]](diffhunk://#diff-39babeb41776345f10ffd90e329b67faf7a8a4f47f0abe7533f665442d52a44cL676-R703) [[3]](diffhunk://#diff-39babeb41776345f10ffd90e329b67faf7a8a4f47f0abe7533f665442d52a44cL693-R754)
  • Loading branch information
paulgessinger authored Oct 11, 2024
1 parent 05c2168 commit 16f6341
Show file tree
Hide file tree
Showing 8 changed files with 193 additions and 103 deletions.
22 changes: 6 additions & 16 deletions Core/include/Acts/Geometry/CylinderVolumeStack.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -85,23 +85,12 @@ class CylinderVolumeStack : public Volume {
/// construction.
/// @param volbounds is the new bounds
/// @param transform is the new transform
/// @pre The volume bounds need to be of type
/// @c CylinderVolumeBounds.
void update(std::shared_ptr<VolumeBounds> volbounds,
std::optional<Transform3> transform = std::nullopt) override;

/// Update the volume bounds and transform. This
/// will update the bounds of all volumes in the stack
/// to accommodate the new bounds and optionally create
/// gap volumes according to the resize strategy set during
/// construction.
/// @param newBounds is the new bounds
/// @param transform is the new transform
/// @param logger is the logger
/// @pre The volume bounds need to be of type
/// @c CylinderVolumeBounds.
void update(std::shared_ptr<CylinderVolumeBounds> newBounds,
std::optional<Transform3> transform, const Logger& logger);
void update(std::shared_ptr<VolumeBounds> volbounds,
std::optional<Transform3> transform = std::nullopt,
const Logger& logger = getDummyLogger()) override;

/// Access the gap volume that were created during attachment or resizing.
/// @return the vector of gap volumes
Expand Down Expand Up @@ -133,11 +122,12 @@ class CylinderVolumeStack : public Volume {
Acts::Logging::Level lvl);

/// Helper function that prints output helping in debugging overlaps
/// @param direction is the overlap check direction
/// @param a is the first volume
/// @param b is the second volume
/// @param logger is the logger
static void overlapPrint(const VolumeTuple& a, const VolumeTuple& b,
const Logger& logger);
static void overlapPrint(BinningValue direction, const VolumeTuple& a,
const VolumeTuple& b, const Logger& logger);

/// Helper function that checks if volumes are properly aligned
/// for attachment.
Expand Down
5 changes: 4 additions & 1 deletion Core/include/Acts/Geometry/Volume.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include "Acts/Geometry/GeometryObject.hpp"
#include "Acts/Utilities/BinningType.hpp"
#include "Acts/Utilities/BoundingBox.hpp"
#include "Acts/Utilities/Logger.hpp"

#include <iosfwd>
#include <memory>
Expand Down Expand Up @@ -83,8 +84,10 @@ class Volume : public GeometryObject {
/// Set the volume bounds and optionally also update the volume transform
/// @param volbounds The volume bounds to be assigned
/// @param transform The transform to be assigned, can be optional
/// @param logger A logger object to log messages
virtual void update(std::shared_ptr<VolumeBounds> volbounds,
std::optional<Transform3> transform = std::nullopt);
std::optional<Transform3> transform = std::nullopt,
const Logger& logger = Acts::getDummyLogger());

/// Construct bounding box for this shape
/// @param envelope Optional envelope to add / subtract from min/max
Expand Down
Loading

0 comments on commit 16f6341

Please sign in to comment.