Skip to content

added NRSSIntegrator with accompanying minor tweaks to WPIntegrator#222

Merged
pbeaucage merged 3 commits intomainfrom
221-unify-q-coordinate-semantics-between-nrss-simulation-outputs-and-pyhyperscattering-reduction-workflows
Apr 14, 2026
Merged

added NRSSIntegrator with accompanying minor tweaks to WPIntegrator#222
pbeaucage merged 3 commits intomainfrom
221-unify-q-coordinate-semantics-between-nrss-simulation-outputs-and-pyhyperscattering-reduction-workflows

Conversation

@delongchamp
Copy link
Copy Markdown
Collaborator

@delongchamp delongchamp commented Apr 8, 2026

Unify NRSS q-coordinate semantics in PyHyperScattering

Summary

This MR adds a new NRSSIntegrator to PyHyperScattering to handle the semantic split between:

  • 2D / reciprocal-plane NRSS outputs, where radial reduction should remain q_perp
  • 3D / detector-aware NRSS outputs, where radial reduction should be relabeled to detector-corrected |q|

The existing WPIntegrator behavior is intentionally preserved. It remains a qx/qy -> chi/q_perp polar remesher and is still correct for reciprocal-plane / 2D NRSS outputs.

Motivation

Current NRSS outputs expose detector-plane qx/qy coordinates. When these are reduced with WPIntegrator, the radial axis is labeled from sqrt(qx^2 + qy^2), which is detector-plane q_perp.

That is semantically fine for 2D NRSS outputs, but it is not the correct geometry-aware radial coordinate for 3D detector-aware NRSS outputs. This MR adds a dedicated reducer for NRSS-style qx/qy data so PyHyperScattering can distinguish those cases explicitly without changing existing WPIntegrator behavior.

What Changed

New NRSSIntegrator

Added PyHyperScattering.NRSSIntegrator, exported through PyHyperScattering.integrate.

Behavior:

  • accepts NRSS-style xarray.DataArray inputs with qx/qy coordinates
  • detects 2D vs 3D semantics from metadata
  • preserves the same angular warp-polar remesh strategy as WPIntegrator
  • uses detector-plane q_perp in 2D mode
  • uses detector-corrected |q| in 3D mode

For 3D mode the corrected radial coordinate is computed from:

q_perp = sqrt(qx^2 + qy^2)
lambda_nm = 1239.84197 / energy_ev
k = 2*pi / lambda_nm
qz = -k + sqrt(k^2 - q_perp^2)
q = sqrt(q_perp^2 + qz^2)

Minimal NRSS metadata contract

The reducer works with detached NRSS result arrays by using:

  • z_dim to distinguish 2D vs 3D semantics
  • phys_size_nm as a self-consistency check against the provided qx/qy coordinates
  • the existing energy coordinate for detector-aware 3D correction

phys_size_nm is not used as an alternate q source. qx/qy remain authoritative.

Strict q-axis self-consistency validation

If phys_size_nm is present, NRSSIntegrator now validates the provided 1D qx and qy axes against the FFT-style detector axes implied by that physical size.

Default behavior is strict:

  • validate_q_coords_against_phys_size="raise" by default
  • "warn" is available
  • False disables the check

This keeps phys_size_nm useful as a detached-result sanity check without creating a second source of truth for q.

WPIntegrator clarification

WPIntegrator was not changed semantically.

Updates made:

  • clarified in the docstring that its radial output is detector-plane q_perp
  • preserved radial_semantics="q_perp" in attrs
  • fixed handling of single unstacked qx/qy images

Tests Added

Added PyHyperScattering unit coverage for:

  • 2D NRSSIntegrator matching WPIntegrator semantics
  • 3D detector-corrected radial coordinate computation
  • fallback to explicit metadata kwargs
  • stack-axis preservation
  • WPIntegrator metadata preservation
  • strict q-axis validation against phys_size_nm
  • warning/disabled validation modes

Docs

Updated integration docs to clarify:

  • WPIntegrator is a qx/qy -> chi/q_perp remesher
  • NRSSIntegrator is the correct reducer for detector-aware 3D NRSS outputs

Verification

Test command run during development:

mamba run -n nrss-pyhyper-dev pytest /homes/deand/dev/PyHyperScattering/tests/test_NRSSIntegrator.py -q

Result:

8 passed

Notes

  • This MR does not change WPIntegrator into a geometry-aware reducer.
  • This MR does not depend on synthetic pyFAI metadata or PFGeneralIntegrator.
  • NRSS-side xarray metadata export was handled separately; this MR focuses on the PyHyperScattering reduction side.
image image

@delongchamp
Copy link
Copy Markdown
Collaborator Author

PyHyperScattering CI Fix Summary

Context

Recent GitHub CI runs for PyHyperScattering started failing after the workflow resolved to newer upstream dependencies, notably:

  • numpy 2.4.4
  • pandas 3.0.2
  • xarray 2026.2.0
  • scipy 1.17.1

The failures were not caused by the new NRSSIntegrator work. They came from three older code paths that depended on legacy upstream behavior.

Root Causes and Fixes

1. Lorentz fitting return objects were not shape-consistent

Files changed:

  • src/PyHyperScattering/Fitting.py
  • tests/test_Fitting_lorentz.py

Problem:

  • fit_lorentz() and fit_lorentz_bg() returned scalar DataArray objects while attaching 1D q coordinates.
  • Under current xarray, this raises a dimension mismatch error.

Fix:

  • Fit results are now returned as arrays broadcast over the original q axis using xr.full_like(...).
  • Failure paths now return NaN arrays with the same shape as the input.

Why this is better:

  • The returned datasets are structurally valid.
  • The results remain directly plottable, for example via res.intensity.plot().
  • The fit output preserves the original coordinate context.

Additional test coverage:

  • Added regression coverage for the original Lorentz fit tests.
  • Added an explicit plotting regression test to confirm the output remains plottable.

2. PyHyper mask loading broke with pandas 3

Files changed:

  • src/PyHyperScattering/PFGeneralIntegrator.py
  • tests/test_PFGeneralIntegrator_mask_parsing.py

Problem:

  • loadPyHyperMask() loaded JSON strings from the mask file and passed them directly to pd.read_json(...).
  • In pandas 3, those strings are interpreted as file paths instead of JSON literals.
  • CI therefore failed with a FileNotFoundError using the raw JSON blob as the filename.

Fix:

  • JSON-string mask payloads are now passed through io.StringIO(...) before pd.read_json(...).
  • Non-string payloads are handled as structured in-memory objects via pd.DataFrame(...).

Why this is better:

  • Compatible with current pandas.
  • Keeps behavior close to the original implementation.
  • Covered by a focused regression test that does not rely on external example-data bundles.

3. Dark-frame selection broke with numpy 2

Files changed:

  • src/PyHyperScattering/SST1RSoXSDB.py
  • tests/test_SST1RSoXSDB_dark_index.py

Problem:

  • Dark subtraction used int(img.dark_id.values).
  • Under numpy 2, converting a length-1 array directly to int raises:
    TypeError: only 0-dimensional arrays can be converted to Python scalars

Initial fix:

  • Replaced direct int(...) coercion with explicit scalarization.

Follow-up hardening:

  • Added _scalarize_dark_index(...) helper.
  • It now:
    • accepts NumPy scalars
    • accepts length-1 arrays
    • accepts longer arrays only if all values are identical
    • raises on empty or non-uniform arrays instead of silently taking the first value

Why this is better:

  • Fixes the current CI failure under numpy 2
  • Fails closed if a future grouping change ever passes inconsistent dark_id values into one subtraction call

Validation Performed

Validation was run in the existing nrss-pyhyper-dev environment, which closely matches the CI dependency set:

  • numpy 2.4.4
  • pandas 3.0.2
  • xarray 2026.2.0
  • scipy 1.17.1
  • pytest 9.0.3

Executed:

cd /homes/deand/dev/PyHyperScattering
PYTHONPATH=src mamba run -n nrss-pyhyper-dev pytest \
  tests/test_Fitting_lorentz.py \
  tests/test_PFGeneralIntegrator_mask_parsing.py \
  tests/test_SST1RSoXSDB_dark_index.py \
  -q

Result:

  • 9 passed

Fit Gut Check Artifact

A visual fit sanity-check plot was generated as below.

This plot overlays the synthetic input data with the recovered Lorentz and Lorentz-plus-background fits. The recovered parameters matched the synthetic inputs.

Suggested MR Description

This MR fixes three PyHyperScattering CI regressions triggered by newer upstream dependencies rather than by the recent NRSS work.

  • Fixed Lorentz fit return objects to preserve the input q shape and remain directly plottable under current xarray.
  • Fixed PyHyper mask parsing for pandas 3 by reading JSON-string payloads through StringIO.
  • Fixed SST1 dark-frame index handling for numpy 2 and hardened the scalarization path to reject non-uniform grouped indices.
  • Added focused regression tests covering all three failure modes.

Targeted verification was run locally in the nrss-pyhyper-dev environment against current dependency versions, and all focused regression tests passed.

image

@delongchamp
Copy link
Copy Markdown
Collaborator Author

Hold off on merge until I fix the NRSSIntegrator on energy stacks - there's a bug there

…includes warp_polar optimization rewrite for full broadcast, ~2-3xx faster
@delongchamp
Copy link
Copy Markdown
Collaborator Author

PyHyperScattering NRSSIntegrator update

Summary

This change updates PyHyperScattering.NRSSIntegrator to fix stacked 3D q-coordinate handling, promote a batched reduction path, and preserve 2D parity with WPIntegrator when the in-memory coordinate order is ("qx", "qy").

What changed

1. Fixed stacked 3D detector-aware q coordinates

Previously, stacked 3D detector-aware reductions could fall back to an index-like q axis (0..N-1) with the physical coordinates stored only in q_abs. In practice this made notebook plots such as:

remeshed_data.sel(energy=285.6, method="nearest").plot()

look like the remesh had lost its physical radial coordinate.

NRSSIntegrator now:

  • keeps a physical q axis for stacked 3D detector-aware reductions,
  • interpolates onto a shared detector-corrected q grid spanning the overlap of all slices,
  • preserves the original per-slice detector-corrected coordinates in q_abs.

The output now advertises:

  • radial_coordinate_mode = "shared_q_grid_interpolated" for this case,
  • radial_semantics = "q_abs_detector_corrected" in 3D detector-aware mode.

2. Promoted a batched integration pathway

The previous implementation reduced stacks slice-by-slice in Python. This session introduced a batched remeshing path and promoted it as the default implementation.

Key points:

  • integrateSingleImage now routes through the batched implementation and squeezes singleton stack dims.
  • integrateImageStack now uses the batched implementation for both default operation and the compatibility aliases method="legacy" / method="batched".
  • The old loop-based stack reduction path was removed from active use.

The batched path:

  • stacks non-spatial dims once,
  • remeshes the full batch in one polar sampling pass,
  • vectorizes detector-corrected q generation across the batch,
  • preserves the same downstream xarray layout and metadata contract.

3. Preserved coordinate-order semantics for 2D parity with WPIntegrator

During refactoring, center handling was updated so that:

  • center_x is always derived from qx,
  • center_y is always derived from qy,
  • the center tuple passed into the remesher follows the actual in-memory spatial dim order.

This means the implementation does not blindly hardcode (center_y, center_x) or (center_x, center_y). Instead it preserves the current image-axis order while ensuring the correct center is associated with each axis.

This was done to preserve 2D-material parity with WPIntegrator in the coordinate ordering we want to treat as authoritative for NRSS/PyHyperScattering interoperability.

Validation performed

Unit tests

tests/test_NRSSIntegrator.py was updated and passes in nrss-pyhyper-dev:

PYTHONPATH=src python -m pytest -q tests/test_NRSSIntegrator.py

Result during this session:

  • 11 passed

New/updated test coverage

The test file now covers:

  • 2D parity with WPIntegrator for the ("qx", "qy") ordering,
  • 3D detector-corrected radial axis behavior,
  • preservation of stack axes,
  • stacked 3D output using a shared physical q axis instead of integer-bin q,
  • parity between the promoted batched path and the compatibility aliases for both 2D and 3D stacks,
  • phys_size_nm / qx,qy consistency validation behavior.

Simple timing check

A simple synthetic timing comparison was run in nrss-pyhyper-dev comparing the previous slice-by-slice logic and the new batched path.

Observed best-case timings from that check were approximately:

  • 2D stack: batched path about 2.9x faster
  • 3D stack: batched path about 2.6x faster

This was a simple local timing check, not a formal benchmark, but it was sufficient to justify promoting the batched implementation.

Files changed

  • /homes/deand/dev/PyHyperScattering/src/PyHyperScattering/NRSSIntegrator.py
  • /homes/deand/dev/PyHyperScattering/tests/test_NRSSIntegrator.py

Notes

  • WPIntegrator itself was not changed in this session.
  • The intentional distinction between WPIntegrator and NRSSIntegrator remains:
    • WPIntegrator reports detector-plane q_perp
    • NRSSIntegrator reports detector-corrected |q| in 3D detector-aware mode
  • The batched implementation is now the active path, while method="legacy" remains accepted as a compatibility alias.

Copy link
Copy Markdown

@pbeaucage-lila pbeaucage-lila left a comment

Choose a reason for hiding this comment

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

lgtm

@pbeaucage pbeaucage merged commit caeb18e into main Apr 14, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unify q-coordinate semantics between NRSS simulation outputs and PyHyperScattering reduction workflows

3 participants