Skip to content

ePIE.reconstruct() runs the reconstruction as it should - #91

Merged
ShantanuKodgirwar merged 2 commits into
mainfrom
fix-epie-reconstruct
Aug 3, 2026
Merged

ePIE.reconstruct() runs the reconstruction as it should#91
ShantanuKodgirwar merged 2 commits into
mainfrom
fix-epie-reconstruct

Conversation

@ShantanuKodgirwar

Copy link
Copy Markdown
Contributor

Second in the series that builds a safety net before performance work. Follows #90.

The bug

ePIE.reconstruct() contained a yield, which makes it a generator function. Calling it
builds a generator object and returns immediately, without running anything.

Most callers call it plainly:

engine_ePIE.reconstruct() # example_scripts/exampleReconstruction_pcPIE.py:126

So those runs have been silently producing an unreconstructed object. The tutorials do the
same.

The fix

reconstruct() now drains the loop to completion. The generator behaviour is kept as
reconstruct_stepwise(), for callers that want to interleave their own work between scan
positions.

Also removed: the unconditional CUDA stream

The position body was wrapped in:

withcp.cuda.Stream(non_blocking=True) asstream:

Two problems:

  • Without CuPy installed, cp is None, so this raises AttributeError. ePIE could not run
    on CPU at all.
  • stream.synchronize() fired after every scan position, forcing a host/device round trip for
    no benefit.

Both are gone.

Test coverage

Five ePIE goldens added: Fraunhofer, ASP, Fresnel, mixed-state (nosm=2, npsm=3), polychrome
(nlambda=3). Suite goes 20 → 30 passing.

Verified the goldens are meaningful by running them against the unfixed engine — all ten fail:

  • the CPU goldens mismatch the recorded arrays;
  • the GPU-vs-CPU comparison raises AttributeError: 'Reconstruction' object has no attribute 'error', because _prepareReconstruction() lives inside the generator body and never ran.

⚠️ Breaking change

Anyone doing for _ in engine.reconstruct(): must switch to reconstruct_stepwise().

Updated in this PR:

filenote
tests/Engines/test_propagator.pygenuinely affected
docs/fpm/overview.mdalready broken on main
docs/advanced/multislice.mdalready broken on main

The two doc snippets iterate the FPM and e3PIE engines, neither of which was ever a generator
for loop, posLoop in engine.reconstruct() would have raised TypeError there today. ePIE was
the only generator in the codebase.

Version

0.2.50.2.6. Not published on its own; grouped with the next two fixes into one release.

ShantanuKodgirwarand others added 2 commits August 3, 2026 12:02
ePIE.reconstruct() contained a `yield`, so it was a generator function.
Calling it built a generator object and returned immediately without doing
any work, which is what most callers do -- including the tutorials and
example_scripts/exampleReconstruction_pcPIE.py:126. Those runs have been
silently producing an unreconstructed object.
reconstruct() now drains the loop to completion. The generator behaviour is
preserved as reconstruct_stepwise(), for callers that want to interleave
their own work between scan positions.
Also removes the unconditional `with cp.cuda.Stream(non_blocking=True)`
around the position body. It made ePIE raise AttributeError on any machine
without CuPy, since `cp` is None there, so the engine could not run on CPU
at all. The per-position stream.synchronize() went with it -- it forced a
host/device round trip after every scan position for no benefit.
Adds five ePIE goldens (Fraunhofer, ASP, Fresnel, mixed-state, polychrome).
Verified they fail against the unfixed engine: the CPU goldens mismatch, and
the GPU-vs-CPU comparison raises AttributeError because _prepareReconstruction()
lives inside the generator body and never ran, so reconstruction.error was
never created.
BREAKING: callers doing `for _ in engine.reconstruct()` must switch to
reconstruct_stepwise(). Updated tests/Engines/test_propagator.py and the two
documentation snippets, which were already broken -- they iterate the FPM and
e3PIE engines, neither of which was ever a generator.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@ShantanuKodgirwarShantanuKodgirwar changed the title Make ePIE.reconstruct() actually run the reconstructionePIE.reconstruct() runs the reconstruction as it shouldAug 3, 2026
@ShantanuKodgirwar
ShantanuKodgirwar merged commit a4d2040 into mainAug 3, 2026
3 checks passed
@ShantanuKodgirwar
ShantanuKodgirwar deleted the fix-epie-reconstruct branch August 3, 2026 10:11
Sign up for freeto 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

@ShantanuKodgirwar