fix: make SBT beam placement frames right-handed

[olantwin]

Problem

Running any Geant4 job on the generated geometry emits 180 CLHEP warnings at initialisation:

ZMxpvImproperRotation: col's X Y and Z supplied form closer to a reflection than a Rotation

The corner-beam and stiffening-frame builders in SBTStructureBuilder.cpp construct the local placement frame with columns (b, n, d) but compute b = d × n. For that column order det[b n d] = (d×n)·(n×d) = −|n×d|² < 0, so the frame is left-handed and every such placement carries a reflection instead of a pure rotation — despite the comment claiming a right-handed frame. GeoModel2G4 forwards these matrices to CLHEP::HepRotation, which warns once per placement: 40 corner beams × 3 pieces + 30 stiffening-frame long beams × 2 flanges = 180.

Fix

Compute b = n × d at both sites so the frame is right-handed.

This does not change the geometry: the placed shapes are GeoBoxes symmetric under local X → −X, and all piece offsets are applied along n (local Y), so negating the local X axis places identical boxes identically.

Verification

• Regenerated ship_geometry.db before/after the fix at the same commit: reflection transforms (det < 0) go from 180 → 0, and the diff shows exactly the 180 formerly-improper transforms with only the first rotation column negated — all 316,418 other transforms are byte-identical (translations, other columns, and all other tables unchanged).
• pixi run test: 39/39 pass, including the overlap check; validate_geometry is clean.
• A/B in aegir (gun_st_geomodel, then fixed_target_mt): old DB produces the warnings, regenerated DB produces none, runs complete normally.

Summary by CodeRabbit

• Bug Fixes
  • Corrected the orientation of H-beam components in decay volume structures.
  • Improved alignment of beam flanges and webs for inclined and longitudinal placements.
  • Ensured generated beam geometry consistently uses the intended right-handed orientation.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: cc2282b8-11b2-4fc0-8697-581cbd38d855

📥 Commits

Reviewing files that changed from the base of the PR and between ca5027d and 4ad1dd8.

📒 Files selected for processing (1)

• subsystems/DecayVolume/src/SBTStructureBuilder.cpp

---

📝 Walkthrough

Walkthrough

The H-beam placement rotation frames now use the opposite cross-product operand order for inclined beams and apply the corresponding orientation update to longitudinal beams.

Changes

Beam rotation-frame orientation

Layer / File(s)	Summary
Update beam rotation frames subsystems/DecayVolume/src/SBTStructureBuilder.cpp	The inclined-beam third axis and longitudinal-beam rotation matrix use the revised cross-product order.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

• ShipSoft/Geometry#33: Updates the same beam-placement rotation-frame calculations in SBTStructureBuilder.cpp.

Suggested reviewers: matclim

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly matches the main change: fixing SBT beam placement frames to be right-handed.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix-sbt-frame-handedness

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[olantwin]

@coderabbitai rate limit

[coderabbitai]

Your plan includes PR reviews subject to rate limits. More reviews will be available in 1 minute.

[olantwin]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[matclim]

Verified the claims against the code rather than the description:

• det = −1 → +1 confirmed for all beam directions including the degenerate branch; the new expression is exactly n × d.
• "Geometry unchanged" confirmed independently: rebuilding all 572 SBT volumes under both conventions gives 572/572 identical occupied regions, 0 moved, and exactly 180 with permuted vertex ordering — matching your count from a completely separate construction.
• Fix is complete: these are the only two hand-built frames in DecayVolume; rotZ in SBTSensorBuilder comes from AngleAxisd and is proper by construction.

Worth stating in the description that negating b rather than n is the only safe fix: offsets are applied along n, so negating n would swap TF and BF, and the inner flange is what bounds the helium in Y.

One suggestion, non-blocking: add a test asserting getXToChildVol(i).linear().determinant() > 0 over the tree. test_decayvolume already walks it for the overlap check, so it's ~10 lines, and it pins this permanently — including for any future builder, given the frame construction is duplicated across both sites.

Also: #68 touches the same file. Should auto-merge, but re-run whichever lands second.