Skip to content

Add DIALS ExperimentList/reflection_table export to every CLI program - #10

Draft
PrinceWalnut wants to merge 18 commits into
neutrons:mainfrom
PrinceWalnut:dials_objects
Draft

Add DIALS ExperimentList/reflection_table export to every CLI program#10
PrinceWalnut wants to merge 18 commits into
neutrons:mainfrom
PrinceWalnut:dials_objects

Conversation

@PrinceWalnut

@PrinceWalnut PrinceWalnut commented Jul 23, 2026

Copy link
Copy Markdown

Summary

Adds a --dials (alias -dials) flag to every subhkl CLI program. When set, a command also writes its output as a DIALS ExperimentList (.expt) and reflection_table (.refl) alongside its native HDF5/MTZ output, so subhkl results can be loaded, inspected, and compared directly against the DIALS suite (dials.image_viewer, dials.show, dials.compare_orientation_matrices, ...). The native output is unchanged, and a missing DIALS install only affects the --dials step.

The export reproduces subhkl's physics (s1 = s0 + S·U·B·h) in the shared lab frame: PolychromaticBeam, one Panel per bank (with refined detector_calibration overrides), A = U·B crystal, one Experiment per frame with the rotation in the setting rotation, off-origin sample handled by a per-frame detector translation, and an ImageSet per frame when pixel data is available.

Commits (logically grouped)

  1. Self-describing HDF5 image container (dials_imageset)
  2. FormatSubhkl dxtbx reader + dxtbx.format entry point
  3. Core HDF5→DIALS converter (dials_export)
  4. --dials flag wired onto all CLI programs
  5. Tests (pure-Python helpers everywhere + DIALS-guarded round-trips)
  6. Documentation (dials_export.rst, index, README)

Verification

Verified in a real DIALS environment (ld-subhkl: conda-forge DIALS/dxtbx 3.29.0, laue-dials and subhkl installed editable). The full export test file passes 29 passed, 1 skipped (skip = the DIALS-absent error path), including the ImageSet round-trip through check_format=True using the entry-point-registered FormatSubhkl.

🤖 Generated with Claude Code

https://claude.ai/code/session_018EK1PTqgmjd8iKg1bC73kg

PrinceWalnut and others added 18 commits July 21, 2026 13:43
@PrinceWalnut @claude
Expose per-peak d_err/ang_err data from compute_metrics
bbba9aa 
Adds return_per_peak option to compute_metrics plus a CSV writer and
histogram plotting helpers, so the raw error distributions can be
inspected instead of only the aggregated median/mean/max stats. Wired
up as --csv and --plot options on the `subhkl metrics` CLI command.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UsgjQGXqZVZBYqxoXvPTwk
@PrinceWalnut @claude
Default --csv/--plot to metrics.csv/metrics.png when given bare
676cbfe 
Splits each into a bool flag plus a --csv-path/--plot-path override.
Click's is_flag=False/flag_value optional-argument trick was tested
against the exact typer==0.21.1/click==8.3.1 pinned in uv.lock and
errors with "Option '--csv' requires an argument", so a single
--csv <path> option can't support "no path" reliably on this stack.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UsgjQGXqZVZBYqxoXvPTwk
@PrinceWalnut @claude
Distinguish true frame index from detector panel in metrics output
9d5d35f 
compute_metrics conflated the two: for Predictor "banks" output, the
value used as "run_index" throughout was actually the composite
frame+panel slot key, so --per-run grouped by panel rather than by
true rotation frame whenever a frame had multiple detector panels.

extract_xyz_from_file now returns panel_index (physical bank) and
frame_index (true frame) alongside the existing slot index used
internally for goniometer/angles and goniometer/R lookups (left
untouched since those arrays are stored per-slot). frame_index is read
from peaks/run_index directly for Indexer/Integrator-format files, and
reconstructed for Predictor output by grouping slots that share an
identical goniometer/angles row (those rows are tiled per-panel from a
single frame's angles at write time, confirmed via source trace).

--per-run now groups by true frame; the per-peak CSV gains a
panel_index column alongside frame_index; the per-run histogram plot
(renamed plot_per_frame_histograms) facets by true frame instead of
panel/slot. Verified against synthetic HDF5 fixtures reproducing the
frame/panel tiling pattern.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UsgjQGXqZVZBYqxoXvPTwk
@PrinceWalnut @claude
Add --bins option to control metrics histogram bin count
4a813be 
plot_error_histograms/plot_per_frame_histograms gain a bins parameter
(default 50/30, matching the previously hardcoded values), threaded
through run_metrics as plot_bins and exposed on the CLI as --bins.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UsgjQGXqZVZBYqxoXvPTwk
@PrinceWalnut @claude
Stop indexer from clobbering peaks/run_index with peaks/image_index
385a65c 
run_index() unconditionally overwrote the true per-peak frame id
(peaks/run_index, correctly written upstream by Peaks.write_hdf5) with
the composite frame+panel slot id (peaks/image_index) whenever the
latter was present in the loaded input data - which is essentially
always for Finder output. This corrupted frame_index downstream (e.g.
in subhkl metrics --per-run) to span n_frames * n_panels instead of
n_frames, and fed the same corrupted value into FindUB.load_from_dict,
which then wrote it right back into the indexed output file.

Only fall back to image_index when run_index is genuinely absent.
Files already indexed with the old behavior need to be re-indexed to
pick up correct peaks/run_index values.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UsgjQGXqZVZBYqxoXvPTwk
@PrinceWalnut @claude
Add self-describing HDF5 image container for DIALS export
07a0eb6 
Introduce subhkl.io.dials_imageset: a small numpy/h5py reader/writer for a
self-describing image container (attrs subhkl_imageset/instrument; float32
frames, panel_ids, frame_ids). This is the pixel-data foundation that lets the
--dials export attach a dxtbx ImageSet without round-tripping geometry through a
NeXus schema.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018EK1PTqgmjd8iKg1bC73kg
@PrinceWalnut @claude
Add FormatSubhkl dxtbx reader and register it via entry point
fe028f0 
FormatSubhkl (subclassing FormatHDF5) reads the subhkl image container and
returns raw pixel data per panel; geometry is intentionally left None so the
exporter's beam/detector/goniometer overrides win. Register it through the
dxtbx.format entry point so a DIALS environment with subhkl installed can open
the exported ImageSets (e.g. in dials.image_viewer).

The entry-point *name* encodes the base class after a colon
("FormatSubhkl:FormatHDF5"), as dxtbx's registry requires; without it the class
is an orphan in the format DAG and check_format=True detection cannot descend
into it.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018EK1PTqgmjd8iKg1bC73kg
@PrinceWalnut @claude
Add HDF5-to-DIALS converter for subhkl outputs
33ce221 
subhkl.io.dials_export builds a DIALS ExperimentList (.expt) and reflection_table
(.refl) from subhkl HDF5 output, reproducing subhkl's physics
(s1 = s0 + S*U*B*h) in the shared lab frame:

- Beam: PolychromaticBeam (incident direction only; per-reflection wavelength in
  the reflection table), falling back to a monochromatic mean-wavelength beam.
- Detector: one Panel per bank from beamlines.json, overridden by a refined
  detector_calibration group when present; metres -> millimetres.
- Crystal: A = U*B from the Busing-Levy B (no 2*pi), space group from the file.
- Goniometer/frames: one Experiment per frame with the rotation baked into the
  setting rotation; per-axis goniometer offsets folded in when reconstructing R.
- Sample position: per-frame detector translated by -R*translation so DIALS'
  origin-mounted sample reproduces subhkl's off-origin geometry; s1 measured from
  xyz - s_lab.
- Images: attaches an ImageSet per frame via the container when pixel data is
  available, else falls back to geometry-only.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018EK1PTqgmjd8iKg1bC73kg
@PrinceWalnut @claude
Add --dials flag to every CLI program
b5e2e73 
Wire a --dials (alias -dials) flag and --dials-prefix option onto all subhkl
commands via a shared _maybe_export_dials helper. When set, each command also
writes its output as a DIALS .expt/.refl pair (named after the primary output,
or the given prefix), passing the appropriate image source so downstream
programs can attach an ImageSet. The native HDF5/MTZ output is unchanged, and a
missing DIALS install produces a clear error for the --dials step only.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018EK1PTqgmjd8iKg1bC73kg
@PrinceWalnut @claude
Add tests for the DIALS export
d8c6bc9 
Cover the pure-Python helpers (output paths, peak collection, detector
calibration, goniometer/sample offsets, image container layout) everywhere, plus
DIALS round-trip tests guarded by importorskip that verify the reflection table,
beam, detector, crystal, goniometer, s1 magnitude, detector calibration, sample
offset, and the ImageSet round-trip through check_format=True.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018EK1PTqgmjd8iKg1bC73kg
@PrinceWalnut @claude
Document the DIALS export
37ebdd3 
Add docs/source/dials_export.rst describing the --dials flag, how subhkl maps
onto the DIALS data model (beam/detector/crystal/goniometer/sample/imageset),
the entry-point registration, and the closest DIALS CLI parallel for each
program. Link it from the docs index and summarize in the README.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018EK1PTqgmjd8iKg1bC73kg
@PrinceWalnut @claude
Test the --dials CLI wiring and handle a missing DIALS gracefully
03a269b 
Add CLI-level coverage for the DIALS export options, which previously had none
(the tests exercised hdf5_to_dials directly): a parametrized help smoke test
asserts every command exposes --dials/--dials-prefix, plus tests for the
disabled no-op and the missing-DIALS path.

Make _maybe_export_dials degrade cleanly when DIALS/dxtbx is not installed: the
program's native output is already written by then, so catch the ImportError
from the lazy toolkit import and warn about the --dials step only instead of
failing the whole command with a traceback.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018EK1PTqgmjd8iKg1bC73kg
@PrinceWalnut @claude
Add conda DIALS CI workflow for the export round-trips
002e21f 
The uv-based unit job runs on PyPI, where dxtbx is unavailable, so the DIALS
round-trip tests are always skipped there. Add a dedicated workflow that
installs DIALS from conda-forge and subhkl (so the dxtbx.format entry point
registers FormatSubhkl), verifies the reader is discoverable, and runs
tests/io/test_dials_export.py. Runs on PRs to main, on demand, and weekly to
catch drift against new DIALS releases; kept separate so it need not gate every
PR.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018EK1PTqgmjd8iKg1bC73kg
@PrinceWalnut @claude
Document how to run the DIALS round-trip tests
4178801 
Add a maintainer note to the DIALS export docs with the conda recipe for
reproducing the DIALS verification environment locally (the ld-subhkl setup the
CI workflow uses), including the FormatSubhkl entry-point check.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018EK1PTqgmjd8iKg1bC73kg
@PrinceWalnut @claude
Open subhkl --dials exports cleanly in the DIALS suite
0d456cd 
Testing the --dials export against a real DIALS/dxtbx install (dials.show,
dials.image_viewer) on CG4D data surfaced several display-level incompatibilities
that kept the exported experiments from opening. Fix them; the 3D geometry used
for the physics is untouched throughout.

- Default to a monochromatic beam at a nominal (peak) wavelength so standard
  DIALS tools, which assume a fixed s0, can open the experiment. subhkl is Laue,
  so the true per-reflection wavelengths stay in the reflection table's
  wavelength column, independent of this beam wavelength -- as laue-dials does.
  A PolychromaticBeam remains available (see the CLI --dials-polychromatic).
- Quantise each panel's pixel size to 1 pm. subhkl metrology stores width and
  height as separate floats agreeing only to ~1e-15 mm, so faithful division
  gave sub-picometre anisotropy that tripped rstbx's exact-equality assert and
  kept multi-panel exports from opening in dials.image_viewer at all.
- Bake a flat 2D montage projection into every panel (a bank-id grid, each panel
  flipped 180 deg in place) so a detector that wraps around the sample renders
  perpendicular in the image viewer instead of as tilted, foreshortened tiles,
  matching the ESS/ISIS neutron formats.
- Set the neutron probe build-independently: the Probe enum is not importable
  from dxtbx.model on every build, so derive it from the beam instance.
- Key bank_to_panel on the true dxtbx panel index rather than the enumerate
  index, which drifted when a bank was absent from the instrument config.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018EK1PTqgmjd8iKg1bC73kg
@PrinceWalnut @claude
Add --dials-polychromatic and --dials-wavelength CLI options
c0cb0b5 
Expose the exported beam model on every subhkl command: --dials-wavelength sets
the monochromatic DIALS beam's nominal (peak) wavelength, and
--dials-polychromatic opts into the faithful Laue PolychromaticBeam (which
standard DIALS tools cannot open). Both thread through _maybe_export_dials into
hdf5_to_dials.

Also make the CLI wiring test robust to Rich's ANSI styling, which interleaves
colour codes inside option names, and assert the two new options are present.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018EK1PTqgmjd8iKg1bC73kg
@PrinceWalnut @claude
Propagate the instrument name into merged image stacks
9ee4e32 
merge-images embedded the unit cell but not the instrument, so the merged file
was not self-describing: its --dials export had no instrument to look up and
produced a detector-less .expt (beam and crystal only). Copy the instrument
attribute from the reduced inputs into the merged output so the export builds the
full detector, and downstream steps need not be told the instrument again.

Found while running the --dials pipeline on real CG4D garnet data.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018EK1PTqgmjd8iKg1bC73kg
@PrinceWalnut @claude
Document how to compare subhkl and DIALS/Laue-DIALS outputs
0841ecb 
Add a "Comparing outputs against DIALS / Laue-DIALS" section: a per-step mapping
to the closest laue.* program, the comparison toolkit (dials.show,
compare_orientation_matrices, image_viewer, reflection_viewer, a flex
reflection-matching snippet), and caveats (wavelength lives in the reflection
table, the stills model, display-only transforms).

Also fix a pre-existing malformed row in the CLI-parallel grid table (one cell
was a character too wide) so the page parses cleanly.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018EK1PTqgmjd8iKg1bC73kg
vivek-booshan
vivek-booshan approved these changes Jul 29, 2026
View reviewed changes

@vivek-booshan vivek-booshan left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll add type info later

@PrinceWalnut
PrinceWalnut marked this pull request as draft July 30, 2026 15:01
@jglaser
jglaser requested review from SmithRWORNL and removed request for SmithRWORNL August 11, 2026 14:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vivek-booshan vivek-booshan vivek-booshan approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@PrinceWalnut @vivek-booshan