Skip to content

Build propagator transfer functions on the device the data is on - #94

Merged
ShantanuKodgirwar merged 2 commits into
mainfrom
fix-propagator-device-placement
Aug 3, 2026
Merged

Build propagator transfer functions on the device the data is on#94
ShantanuKodgirwar merged 2 commits into
mainfrom
fix-propagator-device-placement

Conversation

@ShantanuKodgirwar

Copy link
Copy Markdown
Contributor

Fourth in the series building a safety net before performance work. Follows #90, #91, and the
e3PIE fix.

The bug

Four propagators passed params.gpuSwitch into their cached transfer-function builders instead
of asking where the field actually is:

function line
propagate_twoStepPolychrome Operators.py:271
propagate_scaledASP Operators.py:343
propagate_scaledPolychromeASP Operators.py:415
propagate_polychromeASP Operators.py:495

gpuSwitch is a global intent flag, not a fact about any particular array. Arrays get moved
by BaseEngine._checkGPU and by the engines themselves, independently of when a transfer
function is first built and cached — so the two can disagree. Then a CPU transfer function meets
a GPU field:

TypeError: Unsupported type <class 'numpy.ndarray'>

All four now use isGpuArray(fields). Worth noting the other two transfer-function builders in
the same file (Operators.py:101,147) already did this — these four were the outliers, not
a new convention.

Verification

Each of the four call sites is covered by a GPU-vs-CPU test that passes a device array while
leaving params.gpuSwitch False. Reverting Operators.py to main fails all four:

FAILED test_propagator_gpu_matches_cpu[scaledasp]
FAILED test_propagator_gpu_matches_cpu[polychromeasp]
FAILED test_propagator_gpu_matches_cpu[scaledpolychromeasp]
FAILED test_propagator_gpu_matches_cpu[twosteppolychrome]

Three are polychrome and need nlambda > 1, so they live in tests/regression/ where the
synthetic dataset can supply a spectral density; the single-wavelength ones are in
tests/Operators/.

Other test coverage added

test_object2detector previously called every propagator and asserted nothing. Now:

  • output goldens for all eight propagators
  • round-trip identity for the unitary ones (fraunhofer, fresnel, identity)
  • linearity, P(ax) == aP(x), for all of them

Suite goes 30 → 59 passing.

Why round-trip identity is not asserted for asp/scaledasp

They are band-limited — forward∘inverse discards out-of-band energy, so it is not the identity.
Measured ||P(x)-x||/||x||: 3.4e-01 for asp, 5.4e-02 for scaledasp.

What survives differs between them, which is why neither is asserted:

round-trip vs identity idempotent?
asp 3.4e-01 yes — 3.1e-07, a clean 0/1 cutoff
scaledasp 5.4e-02 no — 1.6e-03, its chirp/rescaling factors are not

Linearity is the property both genuinely share; the numbers themselves are pinned by the
goldens.

Version

0.2.70.2.8, published automatically on merge.

ShantanuKodgirwar and others added 2 commits August 3, 2026 13:27
Four propagators passed params.gpuSwitch into their cached transfer-function
builders instead of asking where the field actually is:

  propagate_twoStepPolychrome     Operators.py:271
  propagate_scaledASP             Operators.py:343
  propagate_scaledPolychromeASP   Operators.py:415
  propagate_polychromeASP         Operators.py:495

The switch is a global intent flag, not a fact about any particular array.
Arrays are moved by BaseEngine._checkGPU and by the engines themselves,
independently of when a transfer function is first built and cached, so the two
can disagree -- and then a CPU transfer function meets a GPU field, or the
reverse:

  TypeError: Unsupported type <class 'numpy.ndarray'>

All four now use isGpuArray(fields). The other transfer-function builders in
this file (Operators.py:101,147) already did exactly this; these four were the
outliers.

Tests. Every one of the four call sites is covered by a GPU-vs-CPU test that
passes a device array while leaving params.gpuSwitch False, and each fails
without this change. Three of them are polychrome and need nlambda > 1, so
they live in tests/regression where the synthetic dataset can supply a
spectral density; the single-wavelength ones are in tests/Operators.

Also adds, for all eight propagators, output goldens; a round-trip identity
test for the unitary ones; and a linearity test for all of them.
test_object2detector previously called every propagator and asserted nothing.

Round-trip identity is asserted only for the unitary propagators. asp and
scaledasp are band-limited -- measured ||P(x)-x||/||x|| is 3.4e-01 and 5.4e-02,
and while asp turns out to be a clean projection (idempotent to 3.1e-07),
scaledasp is not (1.6e-03), so linearity is the property both actually share.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@ShantanuKodgirwar
ShantanuKodgirwar merged commit 294d2ad into main Aug 3, 2026
3 checks passed
@ShantanuKodgirwar
ShantanuKodgirwar deleted the fix-propagator-device-placement branch August 3, 2026 11:41
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.

1 participant