[ESSREDUCE] Generalize cylinder mantle projection to axis-aligned cylinders#633
Open
SimonHeybrock wants to merge 1 commit into
Open
[ESSREDUCE] Generalize cylinder mantle projection to axis-aligned cylinders#633SimonHeybrock wants to merge 1 commit into
SimonHeybrock wants to merge 1 commit into
Conversation
github-actions
Bot
changed the title
Replace `project_onto_cylinder_z` with `project_onto_cylinder`, parametrized
by the cylinder axis ('x', 'y', or 'z') via a right-handed cyclic relabeling
of the in-plane axes. `from_nexus` now accepts the 'cylinder_mantle_y'
projection alongside 'cylinder_mantle_z'.
`make_cylinder_mantle_coords` gains an `axis` keyword defaulting to 'z', so
existing direct calls keep their behavior. Downstream packages can adopt
cylinder-Y by passing `axis='y'` without a synchronized release.
The pixel-noise path is unaffected: it derives from physical pixel-tube
geometry, independent of the projection axis.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
SimonHeybrock
force-pushed
the
generalize-cylinder-projection
branch
from
ab38701 to
75151cd
Compare
nvaytet
reviewed
Jun 10, 2026
| ) | ||
|
|
||
|
|
||
| def test_project_onto_cylinder_y_is_z_result_under_axis_relabeling() -> None: |
Member
There was a problem hiding this comment.
Also add a test for x case, as it is an option above?
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.
Motivation
scipp/esslivedata#970 needs to add projections for MAGIC, which has detectors like DREAM mantle but along a vertical cylinder.
Overview
Generalizes the cylinder-mantle projection in
live.rawfrom a hard-coded z-axis cylinder to any axis-aligned cylinder, motivated by the need for a cylinder-Y projection (vertically-aligned mantle detectors). Only axis-aligned variants are supported, which is sufficient for current detectors.project_onto_cylinder_zbecomesproject_onto_cylinderwith anaxisparameter ('x', 'y', or 'z'), using a right-handed cyclic relabeling of the in-plane axes so the math is shared.from_nexusaccepts the new'cylinder_mantle_y'projection alongside the existing'cylinder_mantle_z'.To avoid forcing a synchronized release with downstream (esslivedata calls
make_cylinder_mantle_coordsdirectly), that function keeps its name and signature, gaining only anaxiskeyword defaulting to'z'. Existing direct calls are unchanged; cylinder-Y is opt-in viaaxis='y'. Thefrom_nexusprojection strings are likewise additive.The pixel-noise path is untouched: it derives from physical pixel-tube geometry, independent of the projection axis.
Note: for
'cylinder_mantle_y'theresolutiondict must key the axial dimension'y'(rather than'z'), since the output coordinate name follows the cylinder axis.Test plan
projectors_test.pystill passes against this branch (directmake_cylinder_mantle_coords(coords)call preserved).