fix: CylVolStack resizing issue #3715
Merged
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
This makes it easier to pass it through if you have one (CylinderVolumeStack mainly)
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
Open
paulgessinger
commented
Oct 11, 2024
andiwand
approved these changes
Oct 11, 2024
|
acts-project-service
added
the
Fails Athena tests
paulgessinger
added a commit
that referenced
this pull request
Oct 12, 2024
This PR makes it such that when a `CylinderVolumeStack` is resized with the gap strategy, if there are already gaps on the outside of the stack, **they are reused** instead of creating extra gaps. Part of to #3502. ``` original size ◀───────────────▶ ┌───────────────┐ │ │ │ │ │ Volume 1 │ │ │ │ │ └───────────────┘ first resize ◀──────────────────────────▶ ┌───────────────┬──────────┐ │ │ │ │ │ │ │ Volume 1 │ Gap │ │ │ │ Gap is │ │ │ reused!──┐ └───────────────┴──────────┘ │ second resize │ ◀───────────────────────────────────▶ │ ┌───────────────┬───────────────────┐ │ │ │ │ │ │ │ │ │ │ Volume 1 │ Gap │◀─────┘ │ │ │ │ │ │ └───────────────┴───────────────────┘ ``` Blocked by: - #3715
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.
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
CylinderVolumeStackandVolumeclasses 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: Added aloggerparameter to theupdatemethod and modified its signature to include this parameter. [1] [2]Core/include/Acts/Geometry/Volume.hpp: Added aloggerparameter to theupdatemethod and included the necessary import for theLoggerclass. [1] [2]Core/src/Geometry/CylinderVolumeStack.cpp: Updated thecommitmethod to include aloggerparameter and modified theinitializeOuterVolumemethod to pass theloggerparameter to theupdatemethod. [1] [2] [3] [4] [5] [6]Improvements to overlap checking:
Core/src/Geometry/CylinderVolumeStack.cpp: Enhanced theoverlapPrintmethod to include adirectionparameter and updated the overlap checking logic to handle different directions more robustly. [1] [2] [3] [4] [5] [6] [7] [8] [9]Volume update improvements:
Core/src/Geometry/CylinderVolumeStack.cpp: Updated theupdatemethod to provide detailed logging of the current and new bounds, and added checks to prevent shrinking the stack size. [1] [2] [3]