feat(imaging): 2D resonance imaging pipeline with two-pass SAMMY fitting (#181)#232
Open
KedoKudo wants to merge 26 commits intofeature/172-2d-imagingfrom
Open
feat(imaging): 2D resonance imaging pipeline with two-pass SAMMY fitting (#181)#232KedoKudo wants to merge 26 commits intofeature/172-2d-imagingfrom
KedoKudo wants to merge 26 commits intofeature/172-2d-imagingfrom
Conversation
Single-function entry point that wires together all imaging pipeline components: loader → orchestrator → aggregator → optional HDF5 save. Includes input validation, parameter forwarding, and 43 unit tests covering wiring, parameter passing, pipeline ordering, and edge cases. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Demonstrates the full pipeline using the test TIFF stack with synthetic fit results: data loading, pixel iteration, configuration, aggregation, abundance map visualization, quality overlays, HDF5 round-trip, and the high-level analyze_imaging() API reference. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace synthetic-only demo with actual pipeline demonstration: - Locate SAMMY executable (PATH or known locations) - Load hyperspectral TIFF and visualize raw data - Configure ImagingConfig for Ta-181 - Run BatchFittingOrchestrator on a 4x4 ROI with real SAMMY execution - Inspect per-pixel fit results (abundances, chi-squared) - Aggregate into 2D maps with ResultsAggregator - Visualize abundance maps and quality overlays - HDF5 save/load round-trip - Demonstrate analyze_imaging() high-level API on a second ROI Requires SAMMY installation to execute. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Two bugs prevented end-to-end 2D imaging with real SAMMY fitting: 1. Typo in lpt_manager.py: `isotope_infomation` (missing 'r') caused pydantic to silently ignore the kwarg, leaving isotope_information=None on all parsed IsotopeParameters. 2. The LPT parser extracts abundances and masses but not isotope names. The aggregator needs names to build per-isotope maps. Added name injection in the orchestrator worker using ImagingConfig.isotopes. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Reverse-engineered the energy axis for the LANL-ORNL test TIFF by matching 16 observed resonance dip positions (bin indices) to known Ta-181 ENDF resonance energies. Linear regression gives: E(eV) = 0.6613 * bin + 13.005 (R^2 = 0.99998) Equivalent to np.linspace(13.0, 343.0, 500). Validation: SAMMY chi-squared drops from 10,112,470 (wrong axis) to 25,424 (correct axis) — a 400x improvement confirming the calibration. Also corrected thickness_mm from 0.127 to 0.025 and energy range in ImagingConfig to match the calibrated grid. Cleared stale outputs. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
For single-isotope fits, SAMMY does not output the "Isotopic abundance and mass for each nuclide" section in the LPT file, leaving the parsed isotopes list empty. This caused the aggregator to fail with "Isotope count mismatch". The worker now builds IsotopeParameters from ImagingConfig when the LPT parser returns no isotopes. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Added reverse-engineered energy grid (13-343 eV) derived by matching resonance dip positions to known Ta-181 ENDF energies. Updated material properties (thickness 0.025 mm) for better SAMMY fit quality. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Identified U-235 as the second isotope in the LANL-ORNL test data by matching all 28 observed resonance dips against ENDF databases. Ta-181 alone explains 22 dips; the remaining 6 (at 53, 71, 73, 154, 299, 320 eV) uniquely match U-235 resonances. Natural W and U-238 leave 3-5 dips unexplained. Updated ImagingConfig to use ["Ta-181", "U-235"] with 50/50 starting abundances and all visualization cells for multi-isotope display. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The SAMMY JSON config was created with adjust="false" for all isotopes, meaning SAMMY treated abundances as fixed and never optimized them. Added _enable_abundance_adjustment() helper that patches the JSON config to set adjust="true" so SAMMY fits the abundance ratio at each pixel. Applied in both the shared staging path and worker fallback. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Removed invalid cmap kwarg from plot_multi_isotope() calls — this parameter is only available on plot_single_isotope(). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The JSON config "adjust" flag controls whether SAMMY modifies ENDF resonance parameters, NOT isotopic abundances. For resonance imaging we must never adjust nuclear cross-sections from ENDF. Reverts the _enable_abundance_adjustment() helper added in 95effb2. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Implements a two-pass SAMMY execution strategy for accurate isotope abundance fitting. Pass 1 (JSON mode) fits global thickness with fixed abundances; Pass 2 (traditional mode with IFLISO=1) fits per-isotope abundances using the thickness from Pass 1. Key changes: - Add _move_broadening_inp_to_par() to resolve SAMMY broadening parameter conflicts between INP and PAR files - Add _enable_abundance_fitting_in_par() to set IFLISO flags - Wire fit_abundances flag through ImagingConfig, SammyFilesMultiMode, and BatchFittingOrchestrator - Add unit tests for new helper functions Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…gions Add spatial stride to iter_pixels() and analyze_imaging() for downsampled iteration over large images. stride=8 on a 256x256 image fits a 32x32 grid (1024 pixels) instead of all 65536. Notebook changes: - Increase n_workers to min(8, cpu_count) for better parallelism - Use full-image with stride=8 instead of small 4x4 ROI - Fix axes iteration bug (axes.flat instead of np.atleast_1d) - Add stride parameter documentation and performance tips Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
KedoKudo
changed the title
Two fixes for the two-pass SAMMY helper functions: 1. _move_broadening_inp_to_par: capture broadening blocks that terminate at EOF without a trailing blank line. Previously the parser only recorded a section when it encountered a blank terminator, silently dropping data if the INP file ended mid-block. 2. _enable_abundance_fitting_in_par: positively identify isotope data lines by parsing atomic mass in cols 1-10 instead of relying on line length. This prevents overwriting columns 31-32 on spin-group continuation lines (after -1 markers) that happen to be >= 32 chars. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Two P1 review fixes: 1. local.py: When fit_abundances=True but SAMNDF.PAR/INP are missing after pass 1, mark the run as failed (success=False) instead of logging a warning and returning success from pass 1 alone. This prevents callers from silently getting fixed-abundance results under a mode documented to vary abundances. 2. orchestrator.py: Only use config-based isotope fallback for genuinely single-isotope fits (where SAMMY omits the isotope section in LPT). For multi-isotope fits, an empty parse result is now surfaced as a PixelFitResult failure instead of being papered over with config defaults, which would produce scientifically incorrect abundance maps. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The test_returns_none_when_space_sufficient test used real disk usage without mocking shutil.disk_usage, causing it to fail on CI runners where consumed disk exceeds the 50 GB test limit. Now mocks disk usage consistently like all other tests in the class. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Two review fixes: P1: Stream pixel spectra instead of materializing a giant list. - analyze_imaging() now passes loader.iter_pixels() directly to fit_pixels() instead of wrapping it in list(). - fit_pixels() accepts Iterable[PixelSpectrum] and materializes internally. - iter_pixels() shares the energy array across all PixelSpectrum objects (no .copy()) since workers get their own copy via pickle. This avoids ~1 GB of duplicated energy arrays for 512x512 images. P2: Expose resolution_file on analyze_imaging(). - Add resolution_file parameter to analyze_imaging() signature. - Forward it to BatchFittingOrchestrator so instrument broadening is available through the high-level API. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove .copy() from transmission and uncertainty in iter_pixels() and get_pixel(). Each PixelSpectrum now holds a lightweight numpy view (~48 bytes) into the shared hyperspectral array instead of an independent copy (~4KB per array). For a 512x512 image with 500 energy bins, this reduces the materialized pixel list from ~2 GB (262K copies x 8KB) to ~52 MB (262K view headers), eliminating OOM risk on 32 GB nodes. Views are safe here because: - The base 3D array stays in memory (referenced by HyperspectralLoader and the hyperspectral variable in analyze_imaging) - No code in the parent process mutates pixel data - Workers receive pixels via pickle, which serializes just the viewed 1D slice regardless of whether it is a view or a copy Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ializing Replace list(pixels) materialization in BatchFittingOrchestrator.fit_pixels() with a callable factory pattern. The factory is invoked twice: a lightweight first pass collects only (row, col) coordinates for validation/deduplication, and a second pass streams PixelSpectrum objects directly to the executor. This avoids holding the entire hyperspectral cube in memory simultaneously. - fit_pixels() now accepts Union[Callable, Iterable] for backwards compat - _submit_pixels stores (row, col) tuples instead of PixelSpectrum objects - _collect_results/_collect_results_with_timeout use coord tuples - Retry rounds re-iterate the factory for only failed coordinates - api.py passes a lambda factory instead of a materialized list - All 44 API tests and 421 imaging tests pass Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…vive teardown When timeout_per_job is set, timed-out workers keep running after executor.shutdown(wait=False), but the ExitStack immediately tears down the shared workspace. Workers using symlinks back into that directory would get ENOENT. Fix: copy shared ENDF files and JSON config into each worker's local temp directory so workers are fully self-contained. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… loading Move TempFileManager() instantiation after HyperspectralLoader.load() so that early failures (missing TIFF, bad config, etc.) don't leave orphaned temp directories on disk. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR implements the complete high-level 2D resonance imaging pipeline for PLEIADES, enabling users to perform pixelwise SAMMY fitting across hyperspectral datasets with a single function call. The implementation introduces a sophisticated two-pass SAMMY fitting strategy where Pass 1 (JSON mode) fits global thickness with fixed abundances, and Pass 2 (traditional mode with IFLISO=1) fits per-isotope abundances using the thickness from Pass 1.
Changes:
- Adds
analyze_imaging()API that orchestrates data loading, parallel fitting, result aggregation, and optional HDF5 export - Implements two-pass SAMMY abundance fitting with helper functions to migrate broadening parameters between passes
- Introduces stride parameter for spatial downsampling (e.g.,
stride=8fits every 8th pixel for 64x faster previews) - Optimizes memory usage with callable pixel factories that stream data without materializing full pixel lists
- Fixes typo in LPT parser (
isotope_infomation→isotope_information) - Adds 8 new test classes with comprehensive coverage of API, orchestrator, and backend changes
Reviewed changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/pleiades/imaging/api.py |
New high-level API with analyze_imaging() function |
src/pleiades/imaging/__init__.py |
Exports analyze_imaging |
src/pleiades/imaging/config.py |
Adds fit_abundances field (default True) |
src/pleiades/imaging/loader.py |
Adds stride parameter and removes unnecessary .copy() calls |
src/pleiades/imaging/orchestrator.py |
Refactors to accept callable pixel factories; adds worker-local ENDF copying; improves multi-isotope LPT parsing |
src/pleiades/sammy/backends/local.py |
Implements two-pass execution with _enable_abundance_fitting_in_par() and _move_broadening_inp_to_par() helpers |
src/pleiades/sammy/interface.py |
Adds fit_abundances field to SammyFilesMultiMode |
src/pleiades/sammy/io/json_manager.py |
Clarifies documentation for adjust field |
src/pleiades/sammy/io/lpt_manager.py |
Fixes typo: isotope_infomation → isotope_information |
tests/unit/pleiades/sammy/backends/test_local.py |
Adds 151 new lines of tests for helper functions |
tests/unit/pleiades/imaging/test_api.py |
Adds 2017 lines of comprehensive TDD-style tests |
tests/unit/pleiades/imaging/test_orchestrator.py |
Adds tests for worker file isolation and multi-isotope parsing |
tests/unit/pleiades/imaging/test_temp_manager.py |
Fixes test to properly mock disk usage |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Expand stride parameter docstring to clarify that output maps retain full (height, width) shape with NaN for unevaluated pixels - Add explicit encoding="utf-8" to all open() calls in local.py helper functions to avoid platform-dependent encoding issues Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Wrap the default TempFileManager in try/except so its base_dir is removed if the orchestrator constructor, fit_pixels, or any later step raises before the manager's __exit__ runs. User-provided managers are left untouched. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Update submodule reference to include documented composition and energy axis for LANL-ORNL_example.tif (U-235 + Pu-241, 1-50 eV). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.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.
Summary
Adds the complete high-level 2D resonance imaging pipeline for PLEIADES, from data loading through isotope abundance mapping.
Key capabilities
analyze_imaging()API — single-call entry point wiringHyperspectralLoader→BatchFittingOrchestrator→ResultsAggregator→ optional HDF5 saveIFLISO=1) fits per-isotope abundances using thickness from Pass 1strideparameter oniter_pixels()andanalyze_imaging()for downsampled iteration over large images (e.g., stride=8 on 256×256 → 1024 pixels)ProcessPoolExecutorwith configurablen_workersChanges
Core pipeline (
src/pleiades/imaging/)api.py—analyze_imaging()with stride, ROI, checkpoint/resume supportloader.py—iter_pixels()with stride parameterconfig.py—ImagingConfigwithfit_abundancesflagorchestrator.py— wiresfit_abundancesthrough to SAMMY backendSAMMY backend (
src/pleiades/sammy/)backends/local.py— two-pass execution,_move_broadening_inp_to_par(),_enable_abundance_fitting_in_par()interface.py—fit_abundancesfield onSammyFilesMultiModeio/json_manager.py— supporting changes for two-pass strategyTests
Notebook (
examples/Notebooks/pleiades_2d_imaging.ipynb)stride=8for full 256×256 image with performance estimation tableaxes.flatinstead ofnp.atleast_1d)Known issues (will iterate in follow-up)
Test plan
pixi run test)jupyter nbconvert --executeCloses #181
🤖 Generated with Claude Code