Skip to content

Stills process imagesequence - #887

Draft
dwmoreau wants to merge 28 commits into
mainfrom
stills_process_imagesequence
Draft

Stills process imagesequence#887
dwmoreau wants to merge 28 commits into
mainfrom
stills_process_imagesequence

Conversation

@dwmoreau

Copy link
Copy Markdown
Contributor

This is a prototype branch, not intended for merge. It serves as a discussion point for what is necessary to convert dials.stills_process from ImageSet to ImageSequence. See the DIALS PR for more information: https://github.com/dials/dials/pull/3181

David Mittan-Moreau and others added 28 commits May 18, 2026 17:47
…quence support

Add XFELImageSequence subclass of ImageSequence with per-frame variable wavelength,
a FormatXFEL mixin that overrides get_imageset() to return XFELImageSequence directly
from format classes, and FormatNXmxXFEL / FormatXTCXFEL concrete subclasses.

XFELImageSequence carries one shared beam/detector model across all frames and adjusts
wavelength per-frame in get_beam(index). A __reduce__ override ensures wavelengths are
preserved through multiprocessing pickling. FormatXTCCspad, FormatXTCJungfrau,
FormatXTCEpix, and FormatXTCRayonix now inherit from FormatXTCXFEL so they produce
XFELImageSequence rather than ImageSet.
Add a length check between wavelengths and indices in
XFELImageSequence.__init__ so a mismatched array fails at construction
rather than deep inside get_beam(index). Hoist `import copy` out of
get_beam's per-frame hot path to the module top.

Also extend FormatXTCXFEL's class docstring to document why understand()
can simply delegate to FormatXTC: XTC streams are exclusively LCLS XFEL
stills (FormatXTC inherits FormatStill via FormatMultiImage), so every
XTC file FormatXTC accepts benefits from XFELImageSequence wrapping.
Adds a C++ XFELBeam class (inherits from Beam) that stores all N per-frame
wavelengths as a single serializable object. stills_process output expt files
now contain one XFELBeam per source file instead of N monochromatic beams.

- beam.h: XFELBeam class; get_wavelength/get_s0 throw (no single value)
- beam.cc: to_dict/from_dict, boost::python binding with shared_ptr holder,
  XFELBeamPickleSuite for multiprocessing support
- beam.py: BeamFactory.make_xfel_beam factory; from_dict dispatch for __id__=xfel
- model/__init__.py: export XFELBeam; inject get_beam_at_frame Python method
- nexus/__init__.py: CachedWavelengthBeamFactory.make_xfel_beam() (single HDF5 read)
- FormatNXmx.py: FormatNXmxXFEL.get_wavelengths() uses make_xfel_beam
- FormatXFEL.py: pass XFELBeam to XFELImageSequence; strip as_imageset/as_sequence
  from kwargs before calling super to avoid assertion failure
- imageset.py: XFELImageSequence carries _xfel_beam; get_beam(None) returns
  XFELBeam; get_beam(i) returns monochromatic Beam using local _wavelengths[i]
A 1-D wavelength array with only one element (shape (1,)) means a
constant wavelength across all frames, not a per-pulse variable-energy
XFEL run.  The previous check (wl.ndim > 0) passed shape-(1,) through
to FormatNXmxXFEL, which then raised in get_wavelengths() because the
scalar guard there covers both shape () and shape (1,).

Add wl.size > 1 to understand() so constant-wavelength NXmx files
(whether stored as a true scalar or as a 1-element array) fall through
to regular FormatNXmx.
…ame scans

When compact_stills_scans=True, to_dict() merges N single-frame stills
scans into one JSON object with frame_numbers and per-frame property
arrays, reducing file size ~8% for typical stills composite output.
Each experiment gains a scan_point index; on load _expand_consolidated_scans
reconstructs the per-frame Scan objects before any other decode logic runs.
…th via XFELBeam.get_monochromatic_beam; allow goniometer=None for still sequences

- imageset.py: remove XFELImageSequence class; inject ImageSequence.get_beam(index)
  Python override that calls XFELBeam.get_monochromatic_beam(wl[i]) when the imageset
  beam is XFELBeam and scan has a "wavelength" property; get_beam() with no index
  returns the shared XFELBeam unchanged.
- FormatXFEL.py: updated to use XFELBeam (lightweight guard type, no wavelength array)
  and stamp per-frame wavelengths into scan.set_property("wavelength", ...).
- FormatXTC.py: FormatXTCXFEL.get_wavelengths() returns per-frame wavelengths from
  beam objects; FormatXTC.understand() now returns bool(ds) instead of True so it
  does not match arbitrary files when psana is available.
- beam.h / beam.cc / beam.py: XFELBeam refined — no wavelength array, get_wavelength()
  and get_s0() raise, operator== compares direction+divergence+polarization only.
  BeamFactory.make_xfel_beam() and from_dict() xfel routing updated accordingly.
- nexus/__init__.py: CachedWavelengthBeamFactory.get_wavelengths() renamed from
  make_xfel_beam(); returns list[float] (Angstrom) instead of XFELBeam.
- Format.py: relax get_imageset() assertion to allow goniometer=None when the scan
  is a zero-oscillation still; rotation sequences still require a goniometer.
…quence=True

Lazy formats (e.g. FormatHDF5SaclaMPCCD sets self.lazy=True) caused
get_imageset(as_sequence=True) to assert "No lazy support for sequences"
because the format instance attribute unconditionally overwrote the
lazy=False default even when the caller had explicitly requested a
sequence.  Guard the override with `and not as_sequence`: if the caller
asks for a sequence, lazy loading is impossible by definition and the
format's lazy preference is irrelevant.
FormatMultiImage.get_imageset: guard all four model-fill conditions with
`and format_instance is not None`. combine_experiments uses check_format=False
so format_instance is always None; stills experiments have goniometer=None,
which triggered the unconditional get_goniometer() call and crashed.
Matches the identical guard already present in Format.get_imageset.

FormatXFEL.get_imageset: when the raw imageset covers only a subset of the
source file's frames (e.g. a composite stills output loaded with explicit
indices), select the matching wavelengths by position rather than assuming
the full wavelength list length equals the imageset length.

ExperimentListDict.decode: when setting the scan on a shared imageset, skip
the set_scan call if the imageset already carries a scan with a "wavelength"
property (written by FormatXFEL). Prevents the JSON-decoded scan from
overwriting the format-provided per-frame wavelength data.
…rite

compact_stills_scans=True collects scan_models in first-encounter order
(the order experiments appear in the ExperimentList).  In composite output
from multiple workers, this is worker order, not frame order, so
frame_numbers and the parallel wavelength array were randomly permuted.

Sort scan_models by image_range[0] before building scan_to_point and the
consolidated JSON object.  scan_point indices in experiment dicts are
recalculated against the sorted order; _expand_consolidated_scans round-trip
is unchanged.
…e stills scans on write

Previously `to_dict()` and `as_json()` accepted `compact_stills_scans=False` and only
wrote the `__stills_consolidated` format when the caller explicitly passed `True`. This
meant ~200+ downstream `.as_file()` / `.as_json()` call sites across dials and cctbx_project
silently produced the old expanded format, breaking any program that processed composite
stills output without knowing to pass the flag.

Remove the parameter and make consolidation automatic. The existing guard (all scans
single-frame with oscillation == 0) ensures rotation data is never affected. Programs that
did not opt in now get compact output for free; programs that explicitly passed
`compact_stills_scans=True` need to drop the kwarg (done in a paired dials commit).
…ndices for still scan sequences

Allow non-contiguous single_file_indices when building an ImageSequence
with a still scan (oscillation width == 0).  Each frame in an XFEL
composite output is an independent shot, so the physical file indices
need not be sequential.  Three blocking assertions removed or
conditioned on scan.is_still():

- imageset.h (C++): sequential-indices assert in ImageSequence constructor
- FormatMultiImage.py: sequential assert in get_imageset() for sequences
- experiment_list.py (_make_sequence): bypass ImageSetFactory.make_sequence()
  when single_file_indices are stored in JSON, calling format_class.get_imageset()
  directly so sparse indices are threaded through rather than replaced with a
  contiguous range derived from the scan image_range.

Together with the stills_process change that builds the shared imageset
with flex.size_t(sorted_frames) instead of range(min_f, max_f+1), this
makes the composite output imageset contain only the integrated frames,
so image_viewer no longer shows non-integrated frames in the chooser.
When decoding a sparse still imageset (non-contiguous single_file_indices,
e.g. composite stills output where some frames failed integration), the
eobj_scan merger builds a scan spanning (min_fn, max_fn) from the
per-experiment scans.  This merged scan has get_num_images() >> N, which
fails FormatMultiImage's assertion scan.get_num_images() <= num_images
(where num_images = len(single_file_indices) = N).

Two fixes in ExperimentListDict:

1. _make_sequence: when single_file_indices is present and the scan is a
   still scan (oscillation width == 0) but its get_num_images() != N,
   replace it with a compact ScanFactory.make_scan_from_properties((1,N),{})
   before passing to format_class.get_imageset().  The per-experiment scans
   (with correct absolute frame numbers and any wavelength properties) are
   stored separately and take precedence for per-experiment beam dispatch.

2. _imageset_from_imageset_data: extend the scan-overwrite guard so that we
   also skip set_scan() when the merged scan spans a range larger than the
   imageset length (sparse stills).  Prevents the imageset scan being
   replaced with the (min_fn, max_fn) scan after get_imageset() returns.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
dials.refine and other consumers that load a composite stills .expt
crashed in StillsReflectionManager with "XFELBeam has no fixed s0":
each Experiment.beam is the shared XFELBeam, which has no wavelength.

Add Experiment.get_monochromatic_beam(): when the beam is an XFELBeam
and the per-frame scan carries a "wavelength" property, return a
monochromatic Beam built from the beam direction + that wavelength.
Any other beam type is returned unchanged, so callers may use it
unconditionally.

ExperimentList.to_dict() now re-consolidates the beam: when the stills
scan consolidation fires and every experiment beam is a plain
monochromatic Beam sharing direction/divergence/polarization (differing
only in wavelength), the N beams are collapsed back to one reconstructed
XFELBeam. The per-frame wavelengths are already written by the scan
consolidation, so nothing is lost; the guard fails and all N beams are
written if any beam was genuinely refined to a different geometry.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…tic_beam

The previous make_xfel_beam accepted only direction and divergence; the
resulting XFELBeam was constructed with the default polarization_fraction=0.5
and lost the instrument values for polarization_normal, flux, transmission,
probe and sample_to_source_distance present on the source beam.
XFELBeam.get_monochromatic_beam then produced a Beam carrying these defaults,
so the Lorentz-polarization correction in integration used 0.5 (vs LCLS ~0.9),
causing systematically wrong intensities.

Extend make_xfel_beam to accept the full polarization tuple (delegating to the
existing C++ XFELBeam full constructor), have FormatXFEL.get_imageset pass the
source-beam values, and have XFELBeam.get_monochromatic_beam forward them when
building the per-frame Beam.

Round-trip verified: a make_xfel_beam(polarization_fraction=0.92) survives
to_dict/from_dict and get_monochromatic_beam unchanged.
get_num_scan_points() previously threw, which made generic Experiment
copy/compare/serialize code (which probes "is this scan-varying?" as a benign
check) require XFEL-awareness.  Return 0 to match the real semantics — XFELBeam
genuinely has no scan points — and let downstream code handle it transparently.

The Python from_dict default for polarization_fraction was 0.999 while the
C++ ctor and BeamFactory.make_xfel_beam used 0.5; align to 0.5 to remove a
silent inconsistency in to_dict/from_dict round-trips that omit the field.
After _expand_consolidated_scans has restored per-frame scans (each with its
own single-element "wavelength" property), replace every Experiment.beam that
is an XFELBeam with the result of Experiment.get_monochromatic_beam().  The
ImageSequence's XFELBeam slot and the scan's "wavelength" property are left
untouched, so per-frame wavelength variations remain available via
imageset.get_beam(i).

This avoids needing to patch every downstream tool individually
(detector_residuals, cctbx.xfel.merge, dials.show, etc.) to call
expt.beam = expt.get_monochromatic_beam() before using the beam.  After this
change, loaded Experiment.beam is always a regular Beam and get_s0() /
get_wavelength() work without XFEL-awareness.
Format.py: restore the `assert goniometer is not None or scan.is_still(), ...`
idiom for the goniometer guard, replacing the if/raise AssertionError form.

FormatNXmxXFEL.understand: wrap the HDF5 open + wavelength check in
try/except so an unexpected exception there returns False (i.e. "I don't
understand this file") instead of aborting format detection.

Remaining hunks: ruff format pass over files reformatted with an older
black version on the branch.  Pure formatting noise, no semantic change.
…nt stills writes

dials.split_experiments emits one .expt per experiment via
ExperimentList([expt]).as_json. The beam re-consolidation block in
to_dict was nested under the scan-consolidation guard (len(scan_models)
> 1) and itself required len(beam_models) > 1, so single-experiment
stills writes serialized __id__: monochromatic with a wavelength on the
beam instead of __id__: xfel with the wavelength on the scan.
Round-tripping splits through dials.combine_experiments then failed.

Compute an all_stills flag once, leave scan consolidation gated on
> 1 (no JSON change for existing multi-experiment writes), and move
beam re-consolidation out so it runs whenever every experiment is a
single-frame still with a "wavelength" scan property and every beam
is a plain Beam with shared geometry. This covers both the
one-experiment-split case and the one-shared-beam case produced by
combine_experiments reference_from_experiment.beam=0.

Also pass polarization_normal, polarization_fraction, flux,
transmission, probe and sample_to_source_distance through to
BeamFactory.make_xfel_beam at re-consolidation - analogous to the
P0.2 load-side passthrough; without this, the write side silently
dropped the LCLS ~0.999 polarization_fraction.
…es, not radians

The C++ properties table stores oscillation in radians, but the JSON
convention (matching to_dict<Scan>) is degrees. The stills-consolidated
write path was dumping the raw radians value, producing a silent unit
mismatch on the round trip. Pull through get_oscillation(deg=True) for
the oscillation / oscillation_width keys.

Adds regression tests covering both a non-zero still oscillation
round-trip and the rotation-scan guard that consolidation must skip.
Run pre-commit (ruff v0.11.12, clang-format v20.1.5) over the .py and
.h/.cc files modified on stills_process_imagesequence. Ruff reformatted
one Python file; clang-format reformatted two C++ files. All branch
files now pass `pre-commit run --files ...`.

No behavior change.
Two ImageSequence-aware fixes informed by a cProfile of dials.stills_process
on a 1448-frame, 256-panel JF16M XFEL run.

1) FormatXFEL.get_imageset constructs its ImageSequence directly instead of
   routing through FormatMultiImage.get_imageset(as_imageset=True). The
   parent's per-frame `for i in single_file_indices: beam[i]=get_beam(i); ...`
   loop was reading ~188k Beam objects only to have the XFEL mixin replace
   every one with a single shared XFELBeam. Local construction must still
   propagate format=cls, vendor, params, template, the reader's num_images
   hint, single_file_indices as flex.size_t, and attach the static pixel
   mask via _add_static_mask_to_iset(fmt, iset). Per-frame wavelength
   variation is preserved by fmt.get_wavelengths() -> Scan "wavelength"
   property -> ImageSequence.get_beam(i) dispatching through
   XFELBeam.get_monochromatic_beam. The subset fix and full polarization
   carry-through (polarization_normal/_fraction, flux, transmission, probe,
   sample_to_source_distance) are preserved verbatim.

2) FormatNXmx._start caches the NXmx tree wrapper, nxdata and nxdetector
   handles, and the 256-tuple of detector-module slices. get_raw_data
   previously called _get_nxmx + walked entries[0].instruments[0].detectors[0]
   per frame and then called get_detector_module_slices(nxdetector) inside
   dxtbx.nexus.get_raw_data, rebuilding the slice tuples over all 256
   modules every frame. Both are pure-function on file-level NXmx metadata.
   dxtbx.nexus.get_raw_data gained an optional module_slices= kwarg with the
   prior recompute as the fallback so non-cached callers are unaffected.

Combined predicted save ~3.2 ks (~25%) on the profiled 12.9 ks wall time.
Move the universal beam geometry (direction, divergence, polarization,
flux, transmission, probe, sample-to-source distance) and its concrete
accessors up into BeamBase, leaving the spectral interface (wavelength /
s0 / scan points) pure-virtual so BeamBase stays abstract. Re-parent
PolychromaticBeam and XFELBeam directly onto BeamBase (was: public Beam):
Beam once again means strictly monochromatic, and the three spectral
representations are direct siblings.

- beam.h: BeamBase gains geometry data + concrete accessors + protected
  (default/parameterized) constructors + concrete operator!=; Beam ctors
  delegate geometry to BeamBase; drop XFELBeam's stray `override` on the
  4-tolerance is_similar_to (no base 4-arg virtual after reparenting).
- beam.cc: bases<Beam> -> bases<BeamBase> for PolychromaticBeam/XFELBeam;
  retype the 7 helpers registered on the BeamBase class_ (get/set
  divergence and sigma_divergence, rotate_around_origin, the two
  set_s0_at_scan_points helpers) from Beam& to BeamBase&. to_dict/
  from_dict bodies and __id__ strings unchanged.
- Python boundary: discover the per-shot resolve capability via
  hasattr(beam, "get_monochromatic_beam") instead of isinstance(XFELBeam)
  in imageset.py, model/__init__.py, experiment_list.py.
- Reparenting makes isinstance(poly/xfel, Beam) False (was True): broaden
  the FormatStill/FormatCBFCspad beam asserts to BeamBase, export BeamBase
  from dxtbx.model.beam, broaden the from_dict hint, fix the .pyi
  PolychromaticBeam parent.
- tests: hierarchy/abstractness, XFEL spectral guards, get_monochromatic_
  beam resolution, and the get_monochromatic_beam capability discriminator.
…ingle_file_indices (P2)

Per-frame identity in __stills_consolidated scans is now stored only as the
imageset's 0-based single_file_indices; the redundant 1-based frame_numbers
array is removed. _expand_consolidated_scans reconstructs each per-frame
image_range from single_file_indices via a per-imageset, deduped-scan_point
zip (multi-lattice frames share a scan_point).

The write side now assigns scan_point per (imageset, unique frame) rather than
per Scan identity. scans() dedups by identity, so multi-lattice experiments
with separate-but-equal Scan objects (the post-split_experiments/combine_-
experiments case, where combine keeps a separate scan per experiment) would
otherwise over-count frames vs the per-frame single_file_indices and make the
file unreadable. This normalizes post-combine output to one entry per frame and
is a no-op for the shared-identity composite path.

Adds a precondition assert that every imageset referenced by a consolidated
experiment is a single-file reader (carries single_file_indices), pointing at
P6. Back-compat for pre-P2 files carrying frame_numbers is deferred to P9
(TODO marker at the read site).
The eager (load_models=True) still-ImageSequence branch built one
experiment per frame but read models with an indexless get_beam(), so
every frame's in-memory expt.beam collapsed to the shared XFELBeam/base
beam -- the ImageSet sibling from_stills_and_crystal already indexes
per-frame. Index per-frame via get_beam(j)/get_detector(j)/
get_goniometer(j), dispatching through the ImageSequence.get_beam
override to the per-frame monochromatic beam.

Note: this corrects the in-memory beam only; persisted per-frame
wavelength is carried by the scan 'wavelength' property, not the beam.
…y_timestamp

consolidate_histories() flattened per-experiment history lines with no
dedup. Combining many same-provenance files (e.g. split->combine) yielded
one identical line per source file, since History has no value eq/hash so
histories() dedups by identity. Flatten through OrderedSet for line-level
dedup, preserving the timestamp sort.

Add an optional history_timestamp to as_json(): when supplied, the appended
provenance line uses that ISO-extended UTC '...Z' string instead of the
current time, so a command writing many files can give them an identical
history line that deduplicates cleanly on recombine.

Part of the P7 split->combine drop audit.
P2 made consolidation reconstruct each frame's image_range from the
imageset's 0-based single_file_indices (frame_numbers removed). Multi-file
CBF stills are non-single-file readers with no single_file_indices, so the
write side's single-file-reader assertion hard-crashed to_dict() and broke
the pre-existing test_experimentlist_imagesequence_stills.

Convert the assertion into an eligibility gate: consolidate only when every
still-scan experiment's imageset is a single-file reader; otherwise fall
through to the normal per-scan write (each image_range emitted, no data
loss). Single-file consolidation still fires. Producing a consolidated
structure for multi-file CBF is P6 proper and remains deferred.
JF16M is XFEL data; FormatNXmxXFEL correctly reads the written file back
as an ImageSequence carrying zero-oscillation still scans (per-frame
wavelength in the scan "wavelength" property). Update the round-trip
assertion from the dead pre-branch contract (XFEL = ImageSet, no scan)
to require the scans be stills; keep the no-goniometer check.
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