Keep GVolume.rotations a list and emit a gemc-parseable rotation string#9
Closed
zhaozhiwen wants to merge 1 commit into
Closed
Keep GVolume.rotations a list and emit a gemc-parseable rotation string#9zhaozhiwen wants to merge 1 commit into
zhaozhiwen wants to merge 1 commit into
Conversation
Two coupled rotation bugs:
- set_rotation() reassigned self.rotations to a plain string while
add_rotation() calls .append(), so set_rotation() followed by
add_rotation() raised AttributeError: 'str' object has no attribute
'append'.
- add_rotation() built a " + "-joined cumulative string
("a,b,c + d,e,f"). gemc's parser (G4ObjectsFactory::getRotation) splits
on commas and only applies a rotation for a single 3-token triple (or
the doubleRotation:/ordered: prefixes), so the "+" form was silently
dropped and the volume placed unrotated — a quiet geometry error.
Keep self.rotations a list everywhere: __init__ starts empty (identity),
set_rotation() stores a single-element list (replacing), add_rotation()
appends a clean triple. get_rotation_string() now emits a form gemc
actually parses: the identity when empty, the single triple for one
rotation, the "doubleRotation:" form for two (already used elsewhere in
this file and supported by the current C++ parser), and fails loudly for
three or more instead of producing a silently-unrotated volume.
Filed as one PR because both issues require the same self.rotations-is-a-
list invariant across __init__/set_rotation/add_rotation/get_rotation_string.
Verified by unit-testing all paths (no rotation, set, add x1/x2/x3, set+add)
under Python 3.12.
Fixes gemc#2
Fixes gemc#3
Contributor
|
this fix has been applied already in the code. |
maureeungaro
added a commit
that referenced
this pull request
Jun 23, 2026
…lignment and sqlite code - this was issue #9
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Two coupled rotation bugs in
GVolume(#2 and #3):set_rotation()reassignedself.rotationsto a plain string whileadd_rotation()calls.append(), soset_rotation()followed byadd_rotation()raisedAttributeError: 'str' object has no attribute 'append'.add_rotation()built a" + "-joined cumulative string ("a,b,c + d,e,f"). gemc's parser (G4ObjectsFactory::getRotation) splits on commas and only applies a rotation for a single 3-token triple (or thedoubleRotation:/ordered:prefixes), so the"+"form was silently dropped and the volume placed unrotated — a quiet geometry error.Keep
self.rotationsa list everywhere:__init__starts empty (identity),set_rotation()stores a single-element list (replacing),add_rotation()appends a clean triple.get_rotation_string()now emits a form gemc actually parses — the identity when empty, the single triple for one rotation, thedoubleRotation:form for two (already used elsewhere in this file and supported by the current C++ parser), and fails loudly for three or more instead of silently producing an unrotated volume.Filed as one PR because both issues require the same
self.rotations-is-a-list invariant across__init__/set_rotation/add_rotation/get_rotation_string— they can't be cleanly separated.Validation: unit-tested every path under Python 3.12 — no rotation (identity),
set_rotation,add_rotation×1 (single triple), ×2 (doubleRotation:), ×3 (loud error), andset_rotationthenadd_rotation(previously crashed, nowdoubleRotation:).Fixes #2
Fixes #3