refactor: consume ptychoml.stitch + dedup pixel-size formula#38
Merged
Garrett Bischof (gwbischof) merged 1 commit intoJun 5, 2026
Merged
Conversation
Reuse generic ptychography code from ptychoml instead of duplicating it (no behavior change): - Delete holoptycho/mosaic_stitch.py; import stitch_batch_into from ptychoml.stitch (NSLS2/ptychoml#10 lifted the algorithms verbatim, so output is bit-identical). - Replace the inline far-field pixel-size formula in streaming_recon.py (gpu_setup) and live_compare_viewer.py with ptychoml.compute_sample_pixel_size — same math. - AGENTS.md: point stitch references at ptychoml.stitch and note the reuse boundary. Correctness of the moved/reused code is covered by ptychoml's tests; the existing holoptycho smoke + streaming_recon tests exercise the new import paths and the gpu_setup pixel-size call. First holoptycho PR (H1) of the #37 decomposition. Co-authored-by: Himanshu Goel <4122621+himanshugoel2797@users.noreply.github.com>
Garrett Bischof (gwbischof)
added a commit
that referenced
this pull request
Jun 8, 2026
ptychoml renamed PtychoViTInference's 'data_is_shifted' parameter to 'fftshift' (None = auto-detect DC via detect_dc_at_corner). holoptycho main still called PtychoViTInference(data_is_shifted=...), which raises TypeError against the pinned ptychoml (the bump in #38 crossed the rename) — the ViT op would crash on its first batch. Smoke tests missed it: the vit_inference import dies earlier on TILED_BASE_URL, and CI envs skip holoscan entirely. Rename PtychoViTInferenceOp's param data_is_shifted -> fftshift and pass fftshift= to the session; drop data_is_shifted=True in ptycho_holo.py and let it default to None (auto-detect), matching PR #37's end-to-end DC handling that pairs with this PR's preprocess_diffraction(fftshift=fftshift_dp=None). Adds tests/test_vit_session_contract.py to pin the holoptycho<->ptychoml call contract so a future rename fails in CI instead of on the beamline. Co-authored-by: Himanshu Goel <4122621+himanshugoel2797@users.noreply.github.com>
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
1 participant
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.
First holoptycho PR of the #37 decomposition (H1). A zero-behavior-change refactor that reuses generic ptychography code from
ptychomlinstead of duplicating it.Changes
ptychoml.stitch. Deleteholoptycho/mosaic_stitch.py; importstitch_batch_intofromptychoml.stitch. The algorithms were lifted verbatim into ptychoml in NSLS2/ptychoml#10 (proven bit-identical on random inputs), so live mosaic output is unchanged.ptychoml.compute_sample_pixel_size. Replace the inlineλ·z / (N·ccd)far-field formula instreaming_recon.py(gpu_setup) andlive_compare_viewer.pywith the shared helper — same math.ptychoml.stitchand document the reuse boundary (generic numpy helpers live in ptychoml; holoptycho keeps the Holoscan operators).Testing
Correctness of the moved/reused code is owned by ptychoml's tests (stitch placement + the pixel-size formula) — not re-tested here. On the holoptycho side the existing suite already exercises the new wiring:
test_streaming_recon.py::TestInitConfigconstructsStreamingPtychoRecon(exercises the newptychomlimport) andTestGpuSetuprunsgpu_setup(exercises thecompute_sample_pixel_sizecalls).test_smoke.pyimportsvit_inference(exercises theptychoml.stitchimport) andstreaming_recon.Depends on
NSLS2/ptychoml#10 (merged).
pixi.lockis bumped to that ptychomlmaincommit.