Skip to content

Stale reconstruction-type = try written to reconstructed HDF5 when running recon_steps --reconstruction-type full #84

Description

@decarlof

Summary

When running tomocupy recon_steps --reconstruction-type full ..., the resulting _rec.h5 file contains two reconstruction-type entries under /process/tomocupy-<version>/, one of which is stale/misleading:

/process/tomocupy-<version>/reconstruction-steps-types/reconstruction-type = full   ← truth (from the CLI)
/process/tomocupy-<version>/reconstruction-types/reconstruction-type       = try    ← default from the OTHER command, never touched

Because the file was produced by a full reconstruction, the second entry (try) is factually wrong and confusing for anyone auditing or diffing the metadata. This matters in practice: we routinely inspect metadata to understand which reconstruction was actually run, and a "try" entry in a _rec.h5 looks like the file might be a try scan when it is not.

At 2-BM, _rec.h5 files are always the result of a full reconstruction. The try recon path writes tiffs (a series of slices at candidate rotation-axis positions for manual inspection), not _rec.h5.

Reproduce

Any invocation via tomocupy recon_steps ... --reconstruction-type full. Example command captured in one of our recent files (also visible as the command attribute on /exchange/data):

tomocupy recon_steps \
  --rotation-axis-auto auto --rotation-axis-method vo \
  --propagation-distance 50.5 --fbp-filter shepp \
  --pixel-size 1.4 --energy 30 \
  --retrieve-phase-method paganin --retrieve-phase-alpha 0.001 \
  --remove-stripe-method vo-all \
  --reconstruction-type full --flat-linear True \
  --vo-all-la-size 201 \
  --file-name /data/.../Pinkshrimp_stained_5x_505mm_test_113.h5

Then:

$ h5dump -d /process/tomocupy-1.1.0/reconstruction-steps-types/reconstruction-type <file>_rec.h5
  → "full"
$ h5dump -d /process/tomocupy-1.1.0/reconstruction-types/reconstruction-type       <file>_rec.h5
  → "try"      ← stale default from the unused `recon` schema

Root cause

src/tomocupy/config.py

Two config sections exist for what is essentially the same setting, one per subcommand:

# config.py:309
SECTIONS['reconstruction-types'] = {
    'reconstruction-type': {'default': 'try', 'choices': ['full', 'try'], ...},
    ...
}
# config.py:322
SECTIONS['reconstruction-steps-types'] = {
    'reconstruction-type': {'default': 'try', 'choices': ['full', 'try', 'try_lamino'], ...},
    ...
}

And two parameter-set tuples, one per subcommand:

# config.py:759
RECON_PARAMS       = (..., 'reconstruction-types',       ...)
RECON_STEPS_PARAMS = (..., 'reconstruction-steps-types', ...)

recon_steps writes its HDF5 with sections=RECON_STEPS_PARAMS (see src/tomocupy/dataio/writer.py:169 and :198).

The writer, update_hdf_process() in config.py:929, loops over every SECTIONS key — not just the sections in the sections= tuple:

# config.py:947–955
for section in SECTIONS:
    ...
    if args and sections and section in sections and hasattr(args, name.replace('-', '_')):
        value = getattr(args, name.replace('-', '_'))   # actual CLI value
    else:
        value = opts['default'] if opts['default'] is not None else ''   # ← stale default lands here

So for the sibling section (reconstruction-types, which is not in RECON_STEPS_PARAMS), the loop falls into the else branch and stamps opts['default'] = 'try' into the HDF5. Symmetrically, if recon (not recon_steps) is used, reconstruction-steps-types/reconstruction-type = 'try' would be written for the same reason.

The behavior is intentional per the docstring:

"If sections are specified, write values from args only to those sections, use the defaults on the remaining ones."

...but writing schema defaults for a section belonging to a different subcommand produces misleading, non-authoritative values in the archived metadata.

Impact

  • Anyone diffing raw vs. reconstructed metadata sees two reconstruction-type values that disagree.
  • Downstream tools that grep for reconstruction-type may pick the wrong one.
  • Reproducibility audits are ambiguous: the archived state suggests either a bug or a mixed-mode run when in fact only full was executed.

Proposed fixes (in order of preference)

  1. Skip sections not in sections=. Simplest: only write the sections the current subcommand actually owns. Change for section in SECTIONS: to for section in (sections or SECTIONS): (and drop the section in sections check inside the loop, since it becomes tautological). Result: _rec.h5 no longer contains the stale sibling group at all.

  2. Write only the subcommand-owned "reconstruction-type" group. Keep the "defaults for other sections" behavior in general, but special-case the two mutually exclusive reconstruction-types / reconstruction-steps-types groups so only the one used by the current subcommand is written.

  3. Change the misleading default. Set the default for reconstruction-type in both sections to something obviously non-authoritative (e.g. 'unused' or the empty string), so a reader can tell the value was never actually set from the CLI. Least invasive but doesn't eliminate the duplication.

Happy to send a PR for option 1 if maintainers agree that's the right direction.

Environment

  • tomocupy version: 1.1.0 (as recorded in the HDF5 group path /process/tomocupy-1.1.0/)
  • Beamline: APS 2-BM

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions