Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Nexus transforms improvements #126

Merged
merged 11 commits into from
Nov 5, 2024
Merged

Nexus transforms improvements #126

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
e53857c
Expose GenericNeXusWorkflow in nexus module
SimonHeybrock Nov 4, 2024
67c7c93
Rename PulseSelection -> TimeInterval
SimonHeybrock Nov 4, 2024
49afdd8
Check earlier for bad time dependence
SimonHeybrock Nov 4, 2024
6619f57
Rename function
SimonHeybrock Nov 4, 2024
97d0ec5
Add mechanism for processing time-dependent transformation chains
SimonHeybrock Nov 4, 2024
cddfcdc
Remove dodgy and likely insufficient time-filter mechanism
SimonHeybrock Nov 4, 2024
dbc2b9c
Match input dim order and offset units
SimonHeybrock Nov 4, 2024
70d9443
Remove tests written during feature development
SimonHeybrock Nov 4, 2024
853544d
Bump scippnexus
SimonHeybrock Nov 5, 2024
63a7d45
spelling
SimonHeybrock Nov 5, 2024
0ce71d5
Explain choice of final bin size
SimonHeybrock Nov 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion src/ess/reduce/nexus/workflow.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,12 @@ def get_calibrated_detector(
da = da.fold(dim="detector_number", sizes=sizes)
# Note: We apply offset as early as possible, i.e., right in this function
# the detector array from the raw loader NeXus group, to prevent a source of bugs.
position = transform.value * snx.zip_pixel_offsets(da.coords)
# If the NXdetector in the file is not 1-D, we want to match the order of dims.
# zip_pixel_offsets otherwise yields a vector with dimensions in the order given
# by the x/y/z offsets.
offsets = snx.zip_pixel_offsets(da.coords).transpose(da.dims).copy()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why copy?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I felt there was little to lose, whereas we still run into some Scipp operations that do not handle non-contiguous data.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I see copy() somewhere, my assumption is that it has some significance. E.g., that the result will be modified in-place. So I went looking but didn't find anything.
Essentially, it increases 'noise' for the reader. But leave it or remove it, whichever you prefer.

# We use the unit of the offsets as this is likely what the user expects.
position = transform.value.to(unit=offsets.unit) * offsets
return CalibratedDetector[RunType](
da.assign_coords(position=position + offset.to(unit=position.unit))
)
Expand Down
39 changes: 33 additions & 6 deletions tests/nexus/workflow_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -289,15 +289,11 @@ def test_get_calibrated_detector_works_if_nexus_component_name_is_missing(


def test_get_calibrated_detector_adds_offset_to_position(
nexus_detector,
transform,
nexus_detector, transform
) -> None:
offset = sc.vector([0.1, 0.2, 0.3], unit='m')
detector = workflow.get_calibrated_detector(
nexus_detector,
offset=offset,
bank_sizes={},
transform=transform,
nexus_detector, offset=offset, bank_sizes={}, transform=transform
)
position = (
compute_component_position(nexus_detector)['data'].coords['position'] + offset
Expand All @@ -306,6 +302,37 @@ def test_get_calibrated_detector_adds_offset_to_position(
assert_identical(detector.coords['position'], position)


def test_get_calibrated_detector_position_dims_matches_data_dims(
nexus_detector, transform
) -> None:
nexus_detector2d = nexus_detector.fold('detector_number', sizes={'y': 2, 'x': 3})
nexus_detector2d['data'].coords['x_pixel_offset'] = sc.linspace(
'x', 0, 1, num=3, unit='m'
)
nexus_detector2d['data'].coords['y_pixel_offset'] = sc.linspace(
'y', 0, 1, num=2, unit='m'
)
offset = sc.vector([0.1, 0.2, 0.3], unit='m')
detector = workflow.get_calibrated_detector(
nexus_detector2d, offset=offset, bank_sizes={}, transform=transform
)
assert detector.sizes == {'y': 2, 'x': 3}
assert detector.coords['position'].sizes == {'y': 2, 'x': 3}


def test_get_calibrated_detector_position_unit_matches_offset_unit(
nexus_detector, transform
) -> None:
nexus_detector['data'].coords['x_pixel_offset'] = (
nexus_detector['data'].coords['x_pixel_offset'].to(unit='mm')
)
offset = sc.vector([0.1, 0.2, 0.3], unit='m')
detector = workflow.get_calibrated_detector(
nexus_detector, offset=offset, bank_sizes={}, transform=transform
)
assert detector.coords['position'].unit == 'mm'


def test_get_calibrated_detector_forwards_coords(nexus_detector, transform) -> None:
nexus_detector['data'].coords['abc'] = sc.scalar(1.2)
detector = workflow.get_calibrated_detector(
Expand Down
Loading