From 13743d4c38658a502075f74760986f504d88a63e Mon Sep 17 00:00:00 2001 From: Pete Jemian Date: Wed, 24 Jun 2026 12:55:39 -0500 Subject: [PATCH 1/8] DOC: issue work happens on a feature branch, not main --- AGENTS.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index f00241b..d54b8ad 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -196,6 +196,12 @@ These are imported from the user's host-wide notes - Do **not** push or open PRs without an explicit request. Commit freely when asked. +- **Issue work happens on a feature branch, never directly on + `main`.** Use an existing feature branch when one already covers + the work (a single branch may span several related issues), or + create one before the first commit. The user may override in rare + cases, but that is not typical; when committing to `main` appears to + be requested, verify first rather than assume. - Issue and PR titles must be **brief** and written for the target audience (beamline staff and users), focusing on the observable problem or change rather than internal implementation detail. Use a From b5a86bf735758727a6e386bccd4b69950c2df5f2 Mon Sep 17 00:00:00 2001 From: Pete Jemian Date: Wed, 24 Jun 2026 10:47:23 -0500 Subject: [PATCH 2/8] eiger_y PV reassigned --- src/id3c/configs/devices.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/id3c/configs/devices.yml b/src/id3c/configs/devices.yml index a41d3fe..f3389d7 100644 --- a/src/id3c/configs/devices.yml +++ b/src/id3c/configs/devices.yml @@ -63,7 +63,7 @@ apstools.devices.motor_factory.mb_creator: class_name: DetectorStage motors: det_x: "3idc:m69" # translation - eiger_y: "3idc:m35" # translation Eiger2 + eiger_y: "3idc:m70" # translation Eiger2 eiger_z: "3idc:m36" # translation Eiger2 # laser_optics axes are interlocked against sample_stage.omega (both From d19b74838903dcd23e1432e89d3c532ee2693a75 Mon Sep 17 00:00:00 2001 From: Pete Jemian Date: Wed, 24 Jun 2026 11:16:41 -0500 Subject: [PATCH 3/8] DOC: comments/docstrings describe current state, not history Add the convention to AGENTS.md: source comments and docstrings describe the code as it is now, for the target audience. Historical narration (used to / formerly / no longer) belongs in issues, PRs, and commit messages, not the source. --- AGENTS.md | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index d54b8ad..a465d38 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -161,6 +161,17 @@ methods, or plan methods on the bundle itself. Example: - D-series docstring rules are enabled (`D100`-`D107`). Every public module, class, and function (including `__init__`) must have a docstring. +- **Comments and docstrings describe the code as it is now**, for the + target audience (beamline staff and users who will maintain and + extend it). Do **not** narrate history in the source: no "used to", + "formerly", "no longer", "the older X", "renamed from", changelog + notes, or migration commentary. That history belongs in GitHub + issues, PRs, and commit messages, which are the repositories for it. + When you change behavior, rewrite the affected comments/docstrings to + describe the new state plainly and delete the obsolete narration + rather than appending to it. Rare exceptions (a non-obvious gotcha a + future reader must know to avoid re-introducing a bug) should be + stated as a present-tense caution, not as a story. ## Documentation conventions From aada23963132fe0c5963bcaaec886e24bec928ab Mon Sep 17 00:00:00 2001 From: Pete Jemian Date: Wed, 24 Jun 2026 11:16:51 -0500 Subject: [PATCH 4/8] FIX: flyscan writes /entry/flyscan_data only from the AD file (#12) The per-frame correlation is now written solely from the authoritative area-detector HDF1 file. When that file is not openable, no group is written and a clear WARNING explains the cause (missing/mis-shaped image-files symlink) and the recovery path. - Merge the per-frame correlation into the single /entry/flyscan_data NXdata group: image_number, frame_index, and timestamp join the image substack and position arrays; the separate /entry/positions group is no longer produced. - Stamp provenance attributes (source, n_frames_paired, n_frames_expected) on /entry/flyscan_data. - Add _expected_frame_count() helper (AD-file count, else start-doc num_frames) with unit tests. - Update the frame-count explanation page to match. --- .../source/explanation/flyscan_frame_count.md | 86 ++-- src/id3c/plans/flyscan_3idc.py | 387 ++++++++++-------- src/id3c/tests/test_expected_frame_count.py | 99 +++++ 3 files changed, 355 insertions(+), 217 deletions(-) create mode 100644 src/id3c/tests/test_expected_frame_count.py diff --git a/docs/source/explanation/flyscan_frame_count.md b/docs/source/explanation/flyscan_frame_count.md index d412cfd..b4a05ea 100644 --- a/docs/source/explanation/flyscan_frame_count.md +++ b/docs/source/explanation/flyscan_frame_count.md @@ -20,39 +20,43 @@ completes, the plan writes the frame-to-position pairing **into the NeXus master HDF5 file** (the `*.hdf` file in your run directory). Look here: -`/entry/positions` (an `NXdata` group) -: The per-frame correlation table, with **one row per in-scan frame** - (the pre-roll and tail frames are already excluded -- so this group - has your 301 rows, not 306). Datasets: - +`/entry/flyscan_data` (an `NXdata` group, the **default plot** and the +single primary product) +: A ready-to-plot view of the in-scan data, with **one row per in-scan + frame** (the pre-roll and tail frames are already excluded -- so this + group has your 301 rows, not 306). Its `data` is a *virtual* dataset + (no bytes copied) that already slices the in-scan 301-frame substack + out of the full image stack, with `position_start_acquire` as its + plot axis. The plan also sets `/entry@default = "flyscan_data"`, so a + NeXus-aware viewer (e.g. NeXpy, h5web) opens this group by default -- + in-scan images vs. omega position, which is the useful default view of + a flyscan. Datasets: + + - `data` -- the in-scan image substack (virtual dataset). + - `position_start_acquire`, `position_end_acquire`, + `position_end_period` -- the motor (omega) position at three + phases of each frame's exposure period (the plot axes). - `image_number` -- IOC frame counter (1-based) for each in-scan frame. - `frame_index` -- `image_number - 1`; the **0-based index into - `/entry/images/data`** for that frame. Use it to pull the in-scan - image substack out of the full (306-frame) image stack: + `/entry/images/data`** for that frame. `data` is already this + substack; use `frame_index` only to map back to the full + (306-frame) image stack: ```python import h5py with h5py.File("20260618-...-S00031-....hdf", "r") as f: - images = f["/entry/images/data"] # all captured frames - idx = f["/entry/positions/frame_index"][:] # in-scan frames only - in_scan_images = images[idx, :, :] # the useful 301 - omega = f["/entry/positions/position_start_acquire"][:] + images = f["/entry/images/data"] # all captured frames + idx = f["/entry/flyscan_data/frame_index"][:] # in-scan frames only + in_scan_images = images[idx, :, :] # the useful 301 + omega = f["/entry/flyscan_data/position_start_acquire"][:] ``` - - `position_start_acquire`, `position_end_acquire`, - `position_end_period` -- the motor (omega) position at three - phases of each frame's exposure period. - `timestamp` -- per-frame timestamp. -`/entry/flyscan_data` (an `NXdata` group, the **default plot**) -: A ready-to-plot view of the same in-scan data. Its `data` is a - *virtual* dataset (no bytes copied) that already slices the in-scan - 301-frame substack out of the full image stack, with - `position_start_acquire` as its plot axis. The plan also sets - `/entry@default = "flyscan_data"`, so a NeXus-aware viewer - (e.g. NeXpy, h5web) opens this group by default -- in-scan images - vs. omega position, which is the useful default view of a flyscan. + Provenance group attributes record that the data came from the + authoritative area-detector file: `source = "ad_file"`, + `n_frames_paired`, and `n_frames_expected`. `/entry/images` (an external link) : An HDF5 external link to the detector IOC's frame file at its @@ -60,17 +64,18 @@ Look here: including the pre-roll and tail frames. This is the dataset whose length is 306, not 301. -So: read `/entry/flyscan_data` (or `/entry/positions`) for the paired, -trimmed result; read `/entry/images` only if you specifically want the -raw superset. +So: read `/entry/flyscan_data` for the paired, trimmed result; read +`/entry/images` only if you specifically want the raw superset. :::{note} -These groups are written best-effort at the end of the run. If the -Tiled catalog hadn't ingested the run yet, or the IOC frame file -wasn't readable from the master-file host, you may see only -`/entry/images` (the external link) with a `WARNING` in the log and no -`/entry/positions` / `/entry/flyscan_data`. In that case re-run the -pairing yourself with the analysis functions linked below. +`/entry/flyscan_data` is written best-effort at the end of the run, and +**only** from the area-detector file. If the Tiled catalog hasn't +ingested the run yet, or the IOC frame file isn't readable from the +master-file host (most commonly: the per-detector image-files symlink +next to the master is missing or mis-shaped), you will see only +`/entry/images` (the external link) with a `WARNING` in the log and +**no** `/entry/flyscan_data`. In that case fix the symlink and re-run +the pairing yourself with the analysis function linked below. ::: :::{admonition} Keep this page in sync with the code @@ -93,11 +98,10 @@ page in the same commit: `hdf_num_capture = int(num_frames * 1.5) + 20` in `flyscan` ([`flyscan_3idc.py`](../../../src/id3c/plans/flyscan_3idc.py)). - Frame-to-position pairing (the thing that selects the useful - frames): `pair_frames_to_positions` and - `pair_frames_to_positions_from_ad_file` in + frames): `pair_frames_to_positions_from_ad_file` in [`flyscan_3idc_analysis.py`](../../../src/id3c/utils/flyscan_3idc_analysis.py). -- Master-file HDF5 layout written at run end (`/entry/positions`, - `/entry/flyscan_data`, `/entry/images`, and `/entry@default`): +- Master-file HDF5 layout written at run end (`/entry/flyscan_data`, + `/entry/images`, and `/entry@default`): `update_master_file` in [`flyscan_3idc.py`](../../../src/id3c/plans/flyscan_3idc.py). If you rename or restructure any of those groups/datasets, update the @@ -194,15 +198,15 @@ Do **not** expect the raw `/entry/data` dataset to equal `num_frames`. The raw file is the superset; the *useful* frames are selected by pairing each frame to the omega position it was exposed at: -- `pair_frames_to_positions(run)` -- from a Bluesky run / Tiled. - `pair_frames_to_positions_from_ad_file(...)` -- directly from the - area-detector HDF5 file. + area-detector HDF5 file (the authoritative, lossless source used by + the plan). -Both live in +It lives in [`flyscan_3idc_analysis.py`](../../../src/id3c/utils/flyscan_3idc_analysis.py). -They use the IOC-timestamped frame stream together with the motor's -position stream to drop the pre-roll and tail frames and attach the -correct omega value to each remaining frame. The result is the 301 +It uses the area-detector file's per-frame timestamps together with the +motor's position stream to drop the pre-roll and tail frames and attach +the correct omega value to each remaining frame. The result is the 301 position-paired frames that span `p_start -> p_end`. ## What this does *not* mean diff --git a/src/id3c/plans/flyscan_3idc.py b/src/id3c/plans/flyscan_3idc.py index acb6dd2..79fdbf9 100644 --- a/src/id3c/plans/flyscan_3idc.py +++ b/src/id3c/plans/flyscan_3idc.py @@ -1401,6 +1401,55 @@ def _wait_for_openable(path, mode="r", retries=5, timeout_s=10.0): return False +def _expected_frame_count( + ad_file_path, + run, + *, + unique_id_dset="/entry/instrument/NDAttributes/NDArrayUniqueId", +): + """Best-effort total acquired-frame count, for provenance. + + Returns the authoritative count from the AD HDF1 file when + ``ad_file_path`` is given and openable (one row per acquired + frame). Otherwise falls back to the scan-derived ``num_frames`` + from the run's start document. Returns ``None`` if neither + source is available. Never raises. + + Note: this is the *total* acquired-frame expectation (including + taxi-in / coast-out frames the AD IOC wrote). The in-scan + paired count can legitimately be smaller; a mismatch is a hint + to inspect, not proof of error. When the source is the lossy + CA-monitor stream, however, a shortfall is the expected failure + mode this provenance is meant to surface. + """ + if ad_file_path is not None: + try: + import h5py + + with h5py.File(ad_file_path, "r") as f: + if unique_id_dset in f: + return int(f[unique_id_dset].shape[0]) + except Exception as exc: # never let provenance break the write + logger.debug( + "_expected_frame_count: could not read %r from AD file %r: %r", + unique_id_dset, + ad_file_path, + exc, + ) + + try: + md = getattr(run, "metadata", None) + start = md["start"] if md is not None else None + if isinstance(start, dict) and start.get("num_frames") is not None: + return int(start["num_frames"]) + except Exception as exc: + logger.debug( + "_expected_frame_count: could not read num_frames from run metadata: %r", + exc, + ) + return None + + # Cam state values (DetectorState_RBV enum) that indicate the cam # failed to arm or is otherwise unusable. Empirically verified on # the Eiger 2026-06-16: failed arm transitions to 'Error' within ~2 ms. @@ -2735,38 +2784,39 @@ def flyscan( def update_master_file(): """Update the NeXus master file once run is complete. - Three updates, in order: + Two updates, in order: 1. Add an HDF5 external link at ``/entry/images`` pointing at the IOC's frame file's ``/entry/data`` group. - 2. Add an ``NXdata`` group at ``/entry/positions`` containing - the per-frame motor-position triple - (``position_start_acquire`` / ``_end_acquire`` / - ``_end_period``) for every in-scan frame, plus - ``image_number`` and ``timestamp`` columns, plus a - ``frame_index`` dataset (= ``image_number - 1``) that - 0-based-indexes into ``/entry/images/data`` along its - first axis (handles the row-count mismatch: the - positions group has only in-scan rows, while the images - dataset has all captured frames including taxi/coast). - Computed by ``flyscan_3idc_analysis.pair_frames_to_positions`` - against the just-completed run. - 3. Add a NeXus-canonical ``NXdata`` group at - ``/entry/flyscan_data`` for default plotting. Group - attributes: ``signal="data"``, ``axes=["position_start_acquire"]``. - ``data`` is a virtual dataset (``h5py.VirtualLayout`` + - ``VirtualSource``) that slices the in-scan substack out - of the externally-linked IOC frame file -- so the bytes - are not copied; the virtual dataset just describes which - rows of the source dataset are in-scan. Sibling - ``position_start_acquire`` / ``position_end_acquire`` / - ``position_end_period`` arrays (duplicated from - ``/entry/positions`` so this group is self-contained for - default plotting). Also sets ``/entry@default = - "flyscan_data"`` so a NeXus-aware viewer opens this group - by default (in-scan images vs. motor position is a more - useful default view of a flyscan run than whatever - apstools NXWriter set originally). + 2. Add the single primary-product ``NXdata`` group at + ``/entry/flyscan_data`` (default plot). Group + attributes: ``signal="data"``, + ``axes=["position_start_acquire"]``, plus provenance + (``source``, ``n_frames_paired``, ``n_frames_expected``). + Contents: + + - ``data``: a virtual dataset (``h5py.VirtualLayout`` + + ``VirtualSource``) that slices the in-scan substack out + of the externally-linked IOC frame file -- so the bytes + are not copied; the virtual dataset just describes which + rows of the source dataset are in-scan. + - ``position_start_acquire`` / ``position_end_acquire`` / + ``position_end_period``: the per-frame motor-position + triple used as plot axes. + - ``image_number`` / ``frame_index`` / ``timestamp``: + subordinate per-frame correlation data. ``frame_index`` + (= ``image_number - 1``) 0-based-indexes into the full + ``/entry/images/data`` stack along its first axis + (handles the row-count mismatch: only in-scan frames are + paired here, while the images dataset has all captured + frames including taxi/coast). + + All contents are computed by + ``flyscan_3idc_analysis.pair_frames_to_positions_from_ad_file`` + against the AD HDF1 file (the only per-frame source). Also + sets ``/entry@default = "flyscan_data"`` so a NeXus-aware + viewer opens this group by default (in-scan images vs. motor + position). Called from ``_main`` after ``takeoff_and_monitor`` returns — i.e. after the inner run_decorator has emitted its stop @@ -2779,10 +2829,10 @@ def update_master_file(): importing them from ``id3c.startup``. If that import fails (id3c not installed, or NEXUS_DATA_FILES.ENABLE was False at startup so ``nxwriter`` was never bound), this - function is a no-op. If only the positions-group write + function is a no-op. If only the flyscan_data-group write fails (e.g. catalog isn't ingestable, or - pair_frames_to_positions raises), the external link is - still written and a WARNING is logged. + pair_frames_to_positions_from_ad_file raises), the external + link is still written and a WARNING is logged. """ # Resolve nxwriter BEFORE any references to it. Doing the # import inside the `if nxwriter is not None:` body (the @@ -2855,184 +2905,169 @@ def update_master_file(): # one row per acquired frame, so no CA-monitor coalescing # gaps). Step 3 also requires the file for the VirtualLayout. external_file_openable = _wait_for_openable(external_file, mode="r") - if not external_file_openable: - logger.warning( - "flyscan._main: external AD HDF1 file %r not openable;" - " step 2 will fall back to CA-monitor source and" - " step 3 will be skipped.", - external_file, - ) - - # Step 2 of 3: /entry/positions per-frame correlation block. - # Skipped (with df=None) if the catalog has no rows yet. - df = None # populated below; needed by step 3 if it runs + # /entry/flyscan_data — the single primary product. One NXdata + # group holding the in-scan image substack (VirtualLayout into + # the external AD file) plus its per-frame correlation data, + # sourced entirely from the AD HDF1 file (the only authoritative, + # lossless per-frame source). If the AD file is not openable we + # write nothing and warn; there is no degraded fallback. + df = None # set below only if the AD file paired successfully try: from id3c.startup import cat - from id3c.utils.flyscan_3idc_analysis import pair_frames_to_positions from id3c.utils.flyscan_3idc_analysis import ( pair_frames_to_positions_from_ad_file, ) uid = nxwriter.uid run = cat[uid] - if external_file_openable: - # Preferred: AD file is authoritative and lossless. - df = pair_frames_to_positions_from_ad_file(run, external_file) - else: - df = pair_frames_to_positions(run) - if len(df) == 0: + if not external_file_openable: logger.warning( - "flyscan._main: pair_frames_to_positions returned" - " 0 rows for uid=%r; skipping steps 2 and 3", - uid, + "flyscan._main: AD file %r is not openable; skipping" + " /entry/flyscan_data. Fix the image-files symlink" + " next to the master file so it resolves, then re-run" + " the analysis to recover /entry/flyscan_data.", + external_file, ) - df = None # disable step 3 too else: - positions_addr = "/entry/positions" + df = pair_frames_to_positions_from_ad_file(run, external_file) + if len(df) == 0: + logger.warning( + "flyscan._main: pair_frames_to_positions_from_ad_file" + " returned 0 rows for uid=%r; skipping" + " /entry/flyscan_data", + uid, + ) + df = None + except Exception as exc: + logger.warning( + "flyscan._main: per-frame pairing failed for" + " /entry/flyscan_data in %s: %r. The group will not" + " be written.", + master_file, + exc, + ) + df = None + + if df is not None: + try: + flyscan_data_addr = "/entry/flyscan_data" # frame_index = image_number - 1: IOC array_counter is # 1-based, HDF5 dataset axes are 0-based. image_number_arr = df["image_number"].to_numpy() frame_index_arr = image_number_arr - 1 + n_frames_paired = int(len(df)) + # Expected total acquired-frame count (authoritative AD + # file row count). The in-scan paired count can be + # legitimately smaller (taxi-in / coast-out frames); + # recorded as provenance for the reader. + n_frames_expected = _expected_frame_count(external_file, run) + with h5py.File(external_file, "r") as src: + src_ds = src[external_addr + "/data"] + src_shape = src_ds.shape # (N, H, W) + src_dtype = src_ds.dtype + # VirtualLayout: out-shape (n_in_scan, H, W), + # each row sourced from src[frame_index[i], :, :]. + n_in_scan = len(frame_index_arr) + out_shape = (n_in_scan,) + tuple(src_shape[1:]) + layout = h5py.VirtualLayout(shape=out_shape, dtype=src_dtype) + vsrc = h5py.VirtualSource( + external_file, + name=external_addr + "/data", + shape=src_shape, + dtype=src_dtype, + ) + for out_i, src_i in enumerate(frame_index_arr): + layout[out_i] = vsrc[int(src_i)] with h5py.File(master_file, "a") as root: - if positions_addr in root: - # Defensive: a re-link attempt (shouldn't - # happen normally) should not crash on - # 'name already exists'. - del root[positions_addr] - grp = root.create_group(positions_addr) - grp.attrs["NX_class"] = "NXdata" - grp.attrs["signal"] = "position_start_acquire" - grp.attrs["axes"] = "image_number" - ds_img = grp.create_dataset("image_number", data=image_number_arr) + if flyscan_data_addr in root: + del root[flyscan_data_addr] + fs_grp = root.create_group(flyscan_data_addr) + fs_grp.attrs["NX_class"] = "NXdata" + fs_grp.attrs["signal"] = "data" + fs_grp.attrs["axes"] = ["position_start_acquire"] + + # Provenance: this group is sourced entirely from the + # authoritative AD HDF1 file. Recorded so a reader + # can confirm the source and frame counts from the + # file alone. + fs_grp.attrs["source"] = "ad_file" + fs_grp.attrs["source_description"] = ( + "Per-frame data read from the authoritative" + " area-detector HDF1 file (lossless, one row per" + " acquired frame)." + ) + fs_grp.attrs["n_frames_paired"] = n_frames_paired + if n_frames_expected is not None: + fs_grp.attrs["n_frames_expected"] = int(n_frames_expected) + + # Update the path to the NeXus default plot. + root["/entry"].attrs["default"] = "flyscan_data" + + # Primary signal: the in-scan image substack. + fs_grp.create_virtual_dataset("data", layout) + + # Plot axes (the position arrays). + fs_grp.create_dataset( + "position_start_acquire", + data=df["position_start_acquire"].to_numpy(), + ) + fs_grp.create_dataset( + "position_end_acquire", + data=df["position_end_acquire"].to_numpy(), + ) + fs_grp.create_dataset( + "position_end_period", + data=df["position_end_period"].to_numpy(), + ) + + # Subordinate per-frame correlation data. + ds_img = fs_grp.create_dataset( + "image_number", data=image_number_arr + ) ds_img.attrs["description"] = ( "IOC-side hdf1.array_counter value at frame" " capture; 1-based per EPICS areaDetector" " NDFileHDF5 plugin convention." ) - ds_idx = grp.create_dataset("frame_index", data=frame_index_arr) + ds_idx = fs_grp.create_dataset("frame_index", data=frame_index_arr) ds_idx.attrs["target"] = "/entry/images/data" ds_idx.attrs["description"] = ( "0-based index into /entry/images/data along" " its first axis; equal to image_number - 1." - " To slice the in-scan image substack:" + " /entry/flyscan_data/data is already this" + " substack; use frame_index only to map back to" + " the full /entry/images/data stack:" " images = f['/entry/images/data'];" - " idx = f['/entry/positions/frame_index'][:];" + " idx = f['/entry/flyscan_data/frame_index'][:];" " in_scan_images = images[idx, :, :]" ) - grp.create_dataset("timestamp", data=df["timestamp"].to_numpy()) - grp.create_dataset( - "position_start_acquire", - data=df["position_start_acquire"].to_numpy(), - ) - grp.create_dataset( - "position_end_acquire", - data=df["position_end_acquire"].to_numpy(), - ) - grp.create_dataset( - "position_end_period", data=df["position_end_period"].to_numpy() - ) + fs_grp.create_dataset("timestamp", data=df["timestamp"].to_numpy()) logger.info( - "flyscan._main: wrote %d per-frame positions" - " (with frame_index 0-based: %d..%d) into NeXus" - " master file %s:%r", - len(df), + "flyscan._main: wrote %s (virtual 'data'" + " shape=%r dtype=%r from %s::%s, %d in-scan frame(s)," + " frame_index 0-based %d..%d) into %s and set" + " /entry@default='flyscan_data'", + flyscan_data_addr, + out_shape, + src_dtype, + external_file, + external_addr + "/data", + n_frames_paired, int(frame_index_arr[0]), int(frame_index_arr[-1]), master_file, - positions_addr, ) - except Exception as exc: - logger.warning( - "flyscan._main: /entry/positions write failed in %s:" - " %r. Step 3 will also be skipped.", - master_file, - exc, - ) - df = None - - # Step 3 of 3: /entry/flyscan_data NXdata + VirtualLayout into - # the external AD HDF1 file. Skip if Loop B above failed. - if df is not None: - if not external_file_openable: - logger.error( - "flyscan._main: external AD HDF1 file %r not" - " openable; skipping /entry/flyscan_data." - " Check that ./ad_files exists and resolves.", - external_file, + except Exception as exc: + logger.warning( + "flyscan._main: failed to write" + " /entry/flyscan_data into NeXus master file %s:" + " %r. The master file is otherwise intact (the" + " external link /entry/images has been written);" + " the default-plot annotation" + " (/entry@default='flyscan_data') is not set.", + master_file, + exc, ) - else: - try: - flyscan_data_addr = "/entry/flyscan_data" - image_number_arr = df["image_number"].to_numpy() - frame_index_arr = image_number_arr - 1 - with h5py.File(external_file, "r") as src: - src_ds = src[external_addr + "/data"] - src_shape = src_ds.shape # (N, H, W) - src_dtype = src_ds.dtype - # VirtualLayout: out-shape (n_in_scan, H, W), - # each row sourced from src[frame_index[i], :, :]. - n_in_scan = len(frame_index_arr) - out_shape = (n_in_scan,) + tuple(src_shape[1:]) - layout = h5py.VirtualLayout(shape=out_shape, dtype=src_dtype) - vsrc = h5py.VirtualSource( - external_file, - name=external_addr + "/data", - shape=src_shape, - dtype=src_dtype, - ) - for out_i, src_i in enumerate(frame_index_arr): - layout[out_i] = vsrc[int(src_i)] - with h5py.File(master_file, "a") as root: - if flyscan_data_addr in root: - del root[flyscan_data_addr] - fs_grp = root.create_group(flyscan_data_addr) - fs_grp.attrs["NX_class"] = "NXdata" - fs_grp.attrs["signal"] = "data" - fs_grp.attrs["axes"] = ["position_start_acquire"] - - # Update the path to the NeXus default plot. - root["/entry"].attrs["default"] = "flyscan_data" - - fs_grp.create_virtual_dataset("data", layout) - # Duplicated from /entry/positions so this - # group is self-contained for default plotting. - fs_grp.create_dataset( - "position_start_acquire", - data=df["position_start_acquire"].to_numpy(), - ) - fs_grp.create_dataset( - "position_end_acquire", - data=df["position_end_acquire"].to_numpy(), - ) - fs_grp.create_dataset( - "position_end_period", - data=df["position_end_period"].to_numpy(), - ) - logger.info( - "flyscan._main: wrote %s (virtual 'data'" - " shape=%r dtype=%r from %s::%s) into %s and set" - " /entry@default='flyscan_data'", - flyscan_data_addr, - out_shape, - src_dtype, - external_file, - external_addr + "/data", - master_file, - ) - except Exception as exc: - logger.warning( - "flyscan._main: failed to write" - " /entry/flyscan_data VirtualLayout into NeXus" - " master file %s: %r. The master file is" - " otherwise intact (external link" - " /entry/images and per-frame correlation" - " /entry/positions have both been written);" - " the default-plot annotation" - " (/entry@default='flyscan_data') is also" - " not set.", - master_file, - exc, - ) def _main(): """Preparation (no data collection) and takeoff (data collection).""" diff --git a/src/id3c/tests/test_expected_frame_count.py b/src/id3c/tests/test_expected_frame_count.py new file mode 100644 index 0000000..d76d684 --- /dev/null +++ b/src/id3c/tests/test_expected_frame_count.py @@ -0,0 +1,99 @@ +"""Tests for ``id3c.plans.flyscan_3idc._expected_frame_count``. + +The helper provides the "expected" frame count stamped as provenance +on ``/entry/flyscan_data`` (issue #12, phase A). It must: + +- prefer the authoritative AD HDF1 file count when a path is given; +- fall back to the start document's ``num_frames`` otherwise; +- return ``None`` when neither source is available; +- never raise. + +Synthetic AD files are built via h5py; runs are duck-typed dicts. +No IOC, no live detector, no catalog. +""" + +from types import SimpleNamespace + +import h5py +import numpy as np + +from id3c.plans.flyscan_3idc import _expected_frame_count + +UID_DSET = "/entry/instrument/NDAttributes/NDArrayUniqueId" + + +def _write_ad_file(path, n_frames): + """Write a minimal AD HDF1 file with ``n_frames`` UID rows.""" + with h5py.File(path, "w") as f: + f.create_dataset(UID_DSET, data=np.arange(n_frames, dtype=np.int32)) + + +def _run_with_num_frames(num_frames): + """Duck-typed run carrying a start document with ``num_frames``.""" + return SimpleNamespace(metadata={"start": {"num_frames": num_frames}}) + + +def test_ad_file_count_is_authoritative(tmp_path): + """When the AD file is openable, its UID row count is returned.""" + ad = tmp_path / "ad.h5" + _write_ad_file(ad, 106) + # Even with a (different) num_frames in metadata, the AD file wins. + run = _run_with_num_frames(101) + assert _expected_frame_count(str(ad), run) == 106 + + +def test_falls_back_to_num_frames_when_no_ad_path(): + """With ad_file_path=None, the start-doc num_frames is used.""" + run = _run_with_num_frames(101) + assert _expected_frame_count(None, run) == 101 + + +def test_falls_back_to_num_frames_when_ad_file_missing(tmp_path): + """A non-openable AD path falls through to num_frames.""" + run = _run_with_num_frames(77) + missing = tmp_path / "does_not_exist.h5" + assert _expected_frame_count(str(missing), run) == 77 + + +def test_falls_back_when_ad_file_lacks_uid_dataset(tmp_path): + """An AD file without the UID dataset falls through to num_frames.""" + ad = tmp_path / "ad_no_uid.h5" + with h5py.File(ad, "w") as f: + f.create_group("/entry") # present, but no NDArrayUniqueId + run = _run_with_num_frames(42) + assert _expected_frame_count(str(ad), run) == 42 + + +def test_returns_none_when_no_source(tmp_path): + """No AD file and no num_frames -> None (not an exception).""" + missing = tmp_path / "nope.h5" + run = SimpleNamespace(metadata={"start": {}}) + assert _expected_frame_count(str(missing), run) is None + + +def test_returns_none_when_num_frames_is_none(): + """An explicit num_frames=None is treated as absent.""" + run = SimpleNamespace(metadata={"start": {"num_frames": None}}) + assert _expected_frame_count(None, run) is None + + +def test_never_raises_on_bad_run_object(): + """A run object with no usable metadata yields None, not an error.""" + assert _expected_frame_count(None, SimpleNamespace()) is None + assert _expected_frame_count(None, None) is None + + +def test_ad_count_used_even_when_metadata_unusable(tmp_path): + """AD file count is returned regardless of a broken run object.""" + ad = tmp_path / "ad.h5" + _write_ad_file(ad, 5) + assert _expected_frame_count(str(ad), None) == 5 + + +def test_returns_plain_int(tmp_path): + """The result is a built-in int (HDF5/JSON attr friendly).""" + ad = tmp_path / "ad.h5" + _write_ad_file(ad, 9) + val = _expected_frame_count(str(ad), None) + # Built-in int, not numpy integer (cleaner HDF5/JSON attr write). + assert isinstance(val, int) and not isinstance(val, np.integer) From 13480f4bd9c095776aa72cef36c4360886b03607 Mon Sep 17 00:00:00 2001 From: Pete Jemian Date: Wed, 24 Jun 2026 11:34:44 -0500 Subject: [PATCH 5/8] FIX: auto-create per-detector image-files symlink (#12) Name the image-files symlink per detector ({det.name}_files, e.g. eiger2_files) so a beamline with several area detectors can give each its own root. flyscan() now creates the symlink next to the master file when the workstation mount (hdf1.read_path_template) is known and exists; it falls back to the descriptive manual-fix warning otherwise and never raises. - Replace fixed AD_FILES_DIRNAME/AD_FILES_ROOT with per-detector ad_files_dirname()/ad_files_root_for() helpers. - Add _ensure_ad_files_symlink() (auto-create, safe fallback) and _read_path_template(); _external_link_target() and _check_ad_files_symlink() use the per-detector name. - Tests updated for the per-detector name; new test_ensure_ad_files_symlink.py. --- src/id3c/plans/flyscan_3idc.py | 117 +++++++++++++---- src/id3c/tests/test_check_ad_files_symlink.py | 34 ++--- .../tests/test_ensure_ad_files_symlink.py | 121 ++++++++++++++++++ src/id3c/tests/test_external_link_target.py | 24 ++-- 4 files changed, 245 insertions(+), 51 deletions(-) create mode 100644 src/id3c/tests/test_ensure_ad_files_symlink.py diff --git a/src/id3c/plans/flyscan_3idc.py b/src/id3c/plans/flyscan_3idc.py index 79fdbf9..4532375 100644 --- a/src/id3c/plans/flyscan_3idc.py +++ b/src/id3c/plans/flyscan_3idc.py @@ -77,17 +77,24 @@ from ophyd.status import SubscriptionStatus from ophyd.utils.errors import WaitTimeoutError -AD_FILES_DIRNAME = "ad_files" -"""Name of the directory/symlink the external link target traverses. -Adjacent to the NeXus master file. -""" +def ad_files_dirname(det): + """Name of the image-files symlink adjacent to the master file. -AD_FILES_ROOT = f"./{AD_FILES_DIRNAME}/" -"""Relative-link root for the master's external link to the AD HDF1 file. + Per detector: ``"{det.name}_files"`` (e.g. ``eiger2_files``). A + beamline with several area detectors can give each its own root. + """ + return f"{det.name}_files" + + +def ad_files_root_for(det): + """Relative-link root for the master's external link to the AD file. + + ``"./{det.name}_files/"``. MUST stay relative (portability). + See ``_external_link_target``. + """ + return f"./{ad_files_dirname(det)}/" -MUST stay relative (portability). See ``_external_link_target``. -""" UNLIMITED_FRAMES = 500_000 """Hard cap on ``cam.num_images`` (Eiger rejects larger values).""" @@ -1274,8 +1281,19 @@ def restore_stage_sigs(snapshot): _AD_FILES_WARNED = set() +def _read_path_template(det): + """Workstation mount path for the detector's image files, or None. + + This is the value the image-files symlink must point at. Sourced + from ``det.hdf1.read_path_template``. + """ + return getattr(det.hdf1, "read_path_template", None) or getattr( + det.hdf1, "_read_path_template", None + ) + + def _check_ad_files_symlink(det, master_dir): - """Warn if ``./ad_files`` is missing in ``master_dir``. + """Warn if the image-files symlink is missing in ``master_dir``. Once per ``(master_dir, target)``. Never creates the link. Returns ``True`` if present, ``False`` if a warning was emitted. @@ -1283,12 +1301,11 @@ def _check_ad_files_symlink(det, master_dir): import os master_dir = str(master_dir) - if os.path.lexists(os.path.join(master_dir, AD_FILES_DIRNAME)): + name = ad_files_dirname(det) + if os.path.lexists(os.path.join(master_dir, name)): return True - read_tmpl = getattr(det.hdf1, "read_path_template", None) or getattr( - det.hdf1, "_read_path_template", None - ) + read_tmpl = _read_path_template(det) if read_tmpl: target = read_tmpl.rstrip("/") or "/" else: @@ -1302,7 +1319,6 @@ def _check_ad_files_symlink(det, master_dir): return False _AD_FILES_WARNED.add(key) - name = AD_FILES_DIRNAME logger.warning( "\n" "The directory or symlink './%s' is missing in %s.\n" @@ -1317,8 +1333,6 @@ def _check_ad_files_symlink(det, master_dir): "\n" " ln -s %s %s\n" "\n" - "The flyscan plan does NOT create this link automatically.\n" - "\n" "To verify:\n" " ls -l %s # should show '%s -> %s'\n" " ls %s/ | head # should list at least one entry", @@ -1336,14 +1350,70 @@ def _check_ad_files_symlink(det, master_dir): return False -def _external_link_target(det, ad_files_root=AD_FILES_ROOT): +def _ensure_ad_files_symlink(det, master_dir): + """Create the image-files symlink in ``master_dir`` if absent. + + The symlink (``{det.name}_files``) maps the relative external-link + root in the master to the workstation mount where the IOC's image + files are visible (``det.hdf1.read_path_template``). Without it, + tools that read the master cannot reach the image data. + + Creates the link when it is safe to do so. Falls back to a + descriptive WARNING (via ``_check_ad_files_symlink``) when it is + not: the target mount is unknown or does not exist, or the link + cannot be created. Never raises. + + Returns ``True`` if the link is present (pre-existing or created), + ``False`` otherwise. + """ + import os + + master_dir = str(master_dir) + name = ad_files_dirname(det) + link_path = os.path.join(master_dir, name) + + if os.path.lexists(link_path): + return True + + target = _read_path_template(det) + if target: + target = target.rstrip("/") or "/" + if not target or not os.path.isdir(target): + # Unknown or non-existent mount: do not guess. Warn instead. + return _check_ad_files_symlink(det, master_dir) + + try: + os.symlink(target, link_path) + except OSError as exc: + logger.warning( + "flyscan: could not create image-files symlink %r -> %r:" + " %r. Falling back to a manual-fix warning.", + link_path, + target, + exc, + ) + return _check_ad_files_symlink(det, master_dir) + + logger.info( + "flyscan: created image-files symlink %r -> %r so the master" + " file's external links resolve to the area-detector images.", + link_path, + target, + ) + return True + + +def _external_link_target(det, ad_files_root=None): """Relative external-link target: ``{ad_files_root}``. + ``ad_files_root`` defaults to the per-detector ``./{det.name}_files/``. ```` is ``det.hdf1.full_file_name`` with ``hdf1.write_path_template`` stripped (the host-specific prefix). Falls back to the full absolute path with a WARNING if the template is missing or doesn't match. """ + if ad_files_root is None: + ad_files_root = ad_files_root_for(det) ioc_file = det.hdf1.full_file_name.get(use_monitor=False) write_tmpl = getattr(det.hdf1, "write_path_template", None) or getattr( det.hdf1, "_write_path_template", None @@ -2855,18 +2925,19 @@ def update_master_file(): import h5py yield from nxwriter.wait_writer_plan_stub() - # Three independent write steps follow. Each is wrapped in - # its own try so one failure does not mask the others. + # Two independent write steps follow (external link, then + # flyscan_data). Each is wrapped in its own try so one failure + # does not mask the other. external_addr = "/entry/data" external_file = _external_link_target(det) master_addr = "/entry/images" master_file = nxwriter.output_nexus_file # AttributeError if not written - import os + from pathlib import Path - _check_ad_files_symlink( - det, master_dir=os.path.dirname(os.path.abspath(master_file)) - ) + # Create the image-files symlink before resolving the external + # link, so the link (and the later AD-file open) resolve. + _ensure_ad_files_symlink(det, master_dir=Path(master_file).absolute().parent) # Loop A: master file openable for append? if not _wait_for_openable(master_file, mode="a"): diff --git a/src/id3c/tests/test_check_ad_files_symlink.py b/src/id3c/tests/test_check_ad_files_symlink.py index d6a1feb..fb86d1a 100644 --- a/src/id3c/tests/test_check_ad_files_symlink.py +++ b/src/id3c/tests/test_check_ad_files_symlink.py @@ -1,8 +1,9 @@ """Tests for ``id3c.plans.flyscan_3idc._check_ad_files_symlink``. Verifies: -- existing ad_files (symlink OR plain dir) is silently accepted; -- missing ad_files emits a WARNING containing the suggested +- existing image-files symlink (symlink OR plain dir) is silently + accepted; +- a missing symlink emits a WARNING containing the suggested ``ln -s`` command and the verification steps; - the warning fires once per (master_dir, target) combination; - a custom read_path_template is reflected in the suggested target; @@ -19,12 +20,14 @@ from id3c.plans.flyscan_3idc import _check_ad_files_symlink -def _make_det(read_path_template="/net/s3data/export/sector3/s3ida/XRD/"): - """Duck-typed det with hdf1.read_path_template.""" +def _make_det( + read_path_template="/net/s3data/export/sector3/s3ida/XRD/", name="eiger2" +): + """Duck-typed det with ``.name`` and hdf1.read_path_template.""" hdf1 = SimpleNamespace() if read_path_template is not None: hdf1.read_path_template = read_path_template - return SimpleNamespace(hdf1=hdf1) + return SimpleNamespace(name=name, hdf1=hdf1) @pytest.fixture(autouse=True) @@ -39,20 +42,20 @@ def test_existing_ad_files_symlink_is_accepted_silently(tmp_path, caplog): """A pre-existing ad_files symlink (any target) means no warning.""" target = tmp_path / "some_mount" target.mkdir() - link = tmp_path / "ad_files" + link = tmp_path / "eiger2_files" link.symlink_to(target) det = _make_det() with caplog.at_level("WARNING", logger="id3c.plans.flyscan_3idc"): ok = _check_ad_files_symlink(det, master_dir=str(tmp_path)) assert ok is True assert not [ - rec for rec in caplog.records if "ad_files" in rec.message - ], "no warning should be emitted when ad_files exists" + rec for rec in caplog.records if "eiger2_files" in rec.message + ], "no warning should be emitted when the symlink exists" def test_existing_ad_files_directory_is_accepted_silently(tmp_path, caplog): - """A plain directory named ad_files (no symlink) is also accepted.""" - (tmp_path / "ad_files").mkdir() + """A plain directory named {det.name}_files (no symlink) is accepted.""" + (tmp_path / "eiger2_files").mkdir() det = _make_det() with caplog.at_level("WARNING", logger="id3c.plans.flyscan_3idc"): ok = _check_ad_files_symlink(det, master_dir=str(tmp_path)) @@ -74,12 +77,11 @@ def test_missing_ad_files_emits_warning_with_ln_s_command(tmp_path, caplog): msg = msgs[0] # Substantive content the operator needs: assert "missing" in msg - assert "ln -s /mnt/data/XRD ad_files" in msg - assert "ls -l ad_files" in msg + assert "ln -s /mnt/data/XRD eiger2_files" in msg + assert "ls -l eiger2_files" in msg assert str(tmp_path) in msg - assert "does NOT create this link automatically" in msg # Helper does NOT actually create the link. - assert not (tmp_path / "ad_files").exists() + assert not (tmp_path / "eiger2_files").exists() def test_warning_fires_only_once_per_master_dir_target_combination(tmp_path, caplog): @@ -120,11 +122,11 @@ def test_missing_read_path_template_uses_placeholder(tmp_path, caplog): assert "the directory on this workstation" in msg assert "where the area-detector files are mounted" in msg # And the placeholder is not literally `/`. - assert "ln -s / ad_files" not in msg + assert "ln -s / eiger2_files" not in msg def test_helper_never_creates_the_symlink(tmp_path): """Pure paranoia: the helper must never create the link.""" det = _make_det() _check_ad_files_symlink(det, master_dir=str(tmp_path)) - assert not (tmp_path / "ad_files").exists() + assert not (tmp_path / "eiger2_files").exists() diff --git a/src/id3c/tests/test_ensure_ad_files_symlink.py b/src/id3c/tests/test_ensure_ad_files_symlink.py new file mode 100644 index 0000000..387f7fc --- /dev/null +++ b/src/id3c/tests/test_ensure_ad_files_symlink.py @@ -0,0 +1,121 @@ +"""Tests for ``id3c.plans.flyscan_3idc._ensure_ad_files_symlink``. + +The plan auto-creates the per-detector image-files symlink +(``{det.name}_files``) next to the master file when it is safe to do +so, and falls back to a descriptive WARNING otherwise. It must never +raise. +""" + +import os +from types import SimpleNamespace + +import pytest + +from id3c.plans import flyscan_3idc as plan_mod +from id3c.plans.flyscan_3idc import _ensure_ad_files_symlink + + +def _make_det(read_path_template, name="eiger2"): + """Duck-typed det with ``.name`` and hdf1.read_path_template.""" + hdf1 = SimpleNamespace() + if read_path_template is not None: + hdf1.read_path_template = read_path_template + return SimpleNamespace(name=name, hdf1=hdf1) + + +@pytest.fixture(autouse=True) +def _clear_warned_set(): + """Reset the once-per-session warned-set between tests.""" + plan_mod._AD_FILES_WARNED.clear() + yield + plan_mod._AD_FILES_WARNED.clear() + + +def test_creates_symlink_when_target_exists(tmp_path): + """A resolvable mount yields a real symlink named {det.name}_files.""" + mount = tmp_path / "mount" + mount.mkdir() + master_dir = tmp_path / "runs" + master_dir.mkdir() + det = _make_det(read_path_template=str(mount)) + + ok = _ensure_ad_files_symlink(det, master_dir=str(master_dir)) + + link = master_dir / "eiger2_files" + assert ok is True + assert link.is_symlink() + assert os.readlink(link) == str(mount) + + +def test_symlink_name_follows_detector_name(tmp_path): + """The link name tracks det.name (e.g. pilatus -> pilatus_files).""" + mount = tmp_path / "mount" + mount.mkdir() + det = _make_det(read_path_template=str(mount), name="pilatus") + + _ensure_ad_files_symlink(det, master_dir=str(tmp_path)) + + assert (tmp_path / "pilatus_files").is_symlink() + + +def test_idempotent_when_link_already_present(tmp_path): + """An existing link of any kind is left untouched and accepted.""" + other = tmp_path / "other" + other.mkdir() + link = tmp_path / "eiger2_files" + link.symlink_to(other) + det = _make_det(read_path_template=str(tmp_path / "new_mount")) + + ok = _ensure_ad_files_symlink(det, master_dir=str(tmp_path)) + + assert ok is True + # Untouched: still points at the original target. + assert os.readlink(link) == str(other) + + +def test_existing_plain_directory_is_accepted(tmp_path): + """A plain directory of the right name is accepted, not replaced.""" + (tmp_path / "eiger2_files").mkdir() + det = _make_det(read_path_template=str(tmp_path / "mount")) + + ok = _ensure_ad_files_symlink(det, master_dir=str(tmp_path)) + + assert ok is True + assert (tmp_path / "eiger2_files").is_dir() + assert not (tmp_path / "eiger2_files").is_symlink() + + +def test_warns_when_target_unknown(tmp_path, caplog): + """No read_path_template -> no link created, a WARNING is emitted.""" + det = _make_det(read_path_template=None) + + with caplog.at_level("WARNING", logger="id3c.plans.flyscan_3idc"): + ok = _ensure_ad_files_symlink(det, master_dir=str(tmp_path)) + + assert ok is False + assert not (tmp_path / "eiger2_files").exists() + assert any(r.levelname == "WARNING" for r in caplog.records) + + +def test_warns_when_target_does_not_exist(tmp_path, caplog): + """A read_path_template that points nowhere -> no link, WARNING.""" + det = _make_det(read_path_template=str(tmp_path / "missing_mount")) + + with caplog.at_level("WARNING", logger="id3c.plans.flyscan_3idc"): + ok = _ensure_ad_files_symlink(det, master_dir=str(tmp_path)) + + assert ok is False + assert not (tmp_path / "eiger2_files").is_symlink() + + +def test_never_raises_on_unwritable_master_dir(tmp_path, caplog): + """A non-existent master_dir (cannot symlink into) -> False, no raise.""" + mount = tmp_path / "mount" + mount.mkdir() + det = _make_det(read_path_template=str(mount)) + missing_dir = tmp_path / "no_such_dir" + + with caplog.at_level("WARNING", logger="id3c.plans.flyscan_3idc"): + ok = _ensure_ad_files_symlink(det, master_dir=str(missing_dir)) + + assert ok is False diff --git a/src/id3c/tests/test_external_link_target.py b/src/id3c/tests/test_external_link_target.py index 609ccf1..0a0f243 100644 --- a/src/id3c/tests/test_external_link_target.py +++ b/src/id3c/tests/test_external_link_target.py @@ -10,26 +10,26 @@ from id3c.plans.flyscan_3idc import _external_link_target -def _make_hdf1(full_file_name, write_path_template=None): - """Build a duck-typed ``det.hdf1`` for the helper.""" +def _make_hdf1(full_file_name, write_path_template=None, name="eiger2"): + """Build a duck-typed ``det`` (with ``.name`` and ``.hdf1``).""" sig = SimpleNamespace(get=lambda use_monitor=False: full_file_name) hdf1 = SimpleNamespace(full_file_name=sig) if write_path_template is not None: hdf1.write_path_template = write_path_template - return SimpleNamespace(hdf1=hdf1) + return SimpleNamespace(name=name, hdf1=hdf1) def test_strips_write_path_template_prefix(): """write_path_template prefix is stripped from full_file_name. - The remaining common suffix is appended to ad_files_root. + The remaining common suffix is appended to the per-detector root. """ det = _make_hdf1( full_file_name="/home/sector3/s3ida/XRD/2026-2/setup/Jun15/foo.h5", write_path_template="/home/sector3/s3ida/XRD/", ) target = _external_link_target(det) - assert target == "./ad_files/2026-2/setup/Jun15/foo.h5" + assert target == "./eiger2_files/2026-2/setup/Jun15/foo.h5" def test_handles_template_without_trailing_slash(): @@ -39,7 +39,7 @@ def test_handles_template_without_trailing_slash(): write_path_template="/home/sector3/s3ida/XRD", # no trailing / ) target = _external_link_target(det) - assert target == "./ad_files/2026-2/setup/Jun15/foo.h5" + assert target == "./eiger2_files/2026-2/setup/Jun15/foo.h5" def test_custom_ad_files_root(): @@ -63,8 +63,8 @@ def test_falls_back_when_template_missing(caplog): ) with caplog.at_level("WARNING", logger="id3c.plans.flyscan_3idc"): target = _external_link_target(det) - # Legacy form: ad_files + the full absolute path (without leading /). - assert target == "./ad_files/home/sector3/s3ida/XRD/2026-2/setup/Jun15/foo.h5" + # Legacy form: per-det root + the full absolute path (no leading /). + assert target == "./eiger2_files/home/sector3/s3ida/XRD/2026-2/setup/Jun15/foo.h5" assert any( "write_path_template not available" in rec.message for rec in caplog.records ) @@ -81,7 +81,7 @@ def test_falls_back_when_template_does_not_prefix_full_file_name(caplog): ) with caplog.at_level("WARNING", logger="id3c.plans.flyscan_3idc"): target = _external_link_target(det) - assert target == "./ad_files/some/other/path/foo.h5" + assert target == "./eiger2_files/some/other/path/foo.h5" assert any("does not prefix" in rec.message for rec in caplog.records) @@ -95,7 +95,7 @@ def test_template_equal_to_full_file_name_yields_empty_suffix(): write_path_template="/home/sector3/s3ida/XRD/", ) target = _external_link_target(det) - assert target == "./ad_files/" + assert target == "./eiger2_files/" def test_uses_underscore_write_path_template_fallback(): @@ -107,6 +107,6 @@ def test_uses_underscore_write_path_template_fallback(): full_file_name=sig, _write_path_template="/home/sector3/s3ida/XRD/", ) - det = SimpleNamespace(hdf1=hdf1) + det = SimpleNamespace(name="eiger2", hdf1=hdf1) target = _external_link_target(det) - assert target == "./ad_files/foo.h5" + assert target == "./eiger2_files/foo.h5" From f826e11be7d2960f6c46f6cdaf56585459031298 Mon Sep 17 00:00:00 2001 From: Pete Jemian Date: Wed, 24 Jun 2026 11:50:43 -0500 Subject: [PATCH 6/8] FIX: stamp path templates + add flyscan master repair tool (#12) Make a flyscan master file self-repairable when /entry/flyscan_data is missing (area-detector file unreachable at run end). - Stamp ad_read_path_template / ad_write_path_template (the IOC->workstation path mapping) into the start metadata so a reader can locate the data files from the master alone. - Extract write_flyscan_data() into flyscan_3idc_analysis so the live plan and the offline tool write an identical /entry/flyscan_data. - Add the id3c-flyscan-repair console script: recompute the pairing from the authoritative area-detector file and write the group. - Add the how-to page and tests for the writer, the CLI, and the path-template helpers. --- docs/source/how_to/index.md | 1 + docs/source/how_to/repair_flyscan_master.md | 82 +++++++ pyproject.toml | 3 + src/id3c/plans/flyscan_3idc.py | 132 +++-------- src/id3c/tests/test_flyscan_repair.py | 201 ++++++++++++++++ src/id3c/tests/test_path_templates.py | 42 ++++ src/id3c/tests/test_write_flyscan_data.py | 117 ++++++++++ src/id3c/utils/flyscan_3idc_analysis.py | 148 ++++++++++++ src/id3c/utils/flyscan_repair.py | 246 ++++++++++++++++++++ 9 files changed, 868 insertions(+), 104 deletions(-) create mode 100644 docs/source/how_to/repair_flyscan_master.md create mode 100644 src/id3c/tests/test_flyscan_repair.py create mode 100644 src/id3c/tests/test_path_templates.py create mode 100644 src/id3c/tests/test_write_flyscan_data.py create mode 100644 src/id3c/utils/flyscan_repair.py diff --git a/docs/source/how_to/index.md b/docs/source/how_to/index.md index 52b4791..a5ba4c6 100644 --- a/docs/source/how_to/index.md +++ b/docs/source/how_to/index.md @@ -8,6 +8,7 @@ Task-oriented recipes. run_a_scan inspect_data visualize_hdf5 +repair_flyscan_master add_a_device add_a_plan edit_and_build_docs diff --git a/docs/source/how_to/repair_flyscan_master.md b/docs/source/how_to/repair_flyscan_master.md new file mode 100644 index 0000000..cd5c7a9 --- /dev/null +++ b/docs/source/how_to/repair_flyscan_master.md @@ -0,0 +1,82 @@ +# Repair a flyscan master file missing `/entry/flyscan_data` + +A flyscan NeXus master file (`*.hdf`) holds its primary product in the +`/entry/flyscan_data` group: the in-scan image substack plus the +per-frame motor positions. That group is written at run end **only** +when the area-detector image file is reachable from the master file's +directory through the image-files symlink (`{detector}_files`, +e.g. `eiger2_files`). + +If the symlink was missing or mis-shaped at run end, the master will +have `/entry/images` (the external link) but **no** +`/entry/flyscan_data`, and a `WARNING` will be in the run log. Once +the symlink is fixed, recover the group with the repair tool. + +## Step 1 -- fix the image-files symlink + +From the directory that contains the master file, create the symlink +pointing at the workstation mount where the detector's image files are +visible. For an `eiger2` detector whose files are mounted at +`/net/s3data/export/sector3/s3ida/XRD/`: + +```bash +cd /path/to/your/run/directory +ln -s /net/s3data/export/sector3/s3ida/XRD/ eiger2_files +ls -l eiger2_files # should show 'eiger2_files -> .../XRD/' +ls eiger2_files/ | head # should list at least one entry +``` + +The symlink must map directly to the image-file root. A common +mistake is creating a directory `eiger2_files/` that *contains* a +child symlink (e.g. `eiger2_files/XRD -> .../XRD`); that adds an extra +path level and the external link will not resolve. + +## Step 2 -- run the repair tool + +```bash +id3c-flyscan-repair /path/to/your/run/20260618-...-S00024-....hdf +``` + +The tool: + +1. reads the run uid from the master's `/entry/entry_identifier`; +2. locates the area-detector file from the master's `/entry/images` + external link (or, if absent, composes it from the `ad_file_path` / + `ad_file_name` start metadata); +3. recomputes the per-frame pairing from the authoritative + area-detector file; and +4. writes `/entry/flyscan_data` (replacing any existing copy) and sets + `/entry@default = "flyscan_data"`. + +### Preview without writing + +```bash +id3c-flyscan-repair --dry-run /path/to/.../master.hdf +``` + +### Point at a specific area-detector file + +If the master cannot resolve the area-detector path (or you want to +override it), pass it explicitly: + +```bash +id3c-flyscan-repair --external-file /net/.../XRDS4_..._000001.h5 \ + /path/to/.../master.hdf +``` + +Add `-v` for INFO-level logging. + +## When repair is not possible + +The tool exits non-zero and explains the cause if: + +- the master has no run uid (`/entry/entry_identifier`); +- the area-detector file cannot be located or does not resolve (most + often the symlink from Step 1 is still wrong); +- the area-detector file is not openable; or +- pairing produces zero in-scan frames. + +The repair reads only the area-detector file (the lossless, +authoritative per-frame source). It never falls back to a lossy +source, so a repaired `/entry/flyscan_data` is always the full-count +result. diff --git a/pyproject.toml b/pyproject.toml index daa198f..d39656e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -50,6 +50,9 @@ line-length = 88 indent-width = 4 target-version = "py311" +[project.scripts] +id3c-flyscan-repair = "id3c.utils.flyscan_repair:main" + [project.optional-dependencies] dev = [ "build", "isort", "mypy", "pre-commit", "pytest", "ruff",] doc = [ "babel", "ipykernel", "jinja2", "linkify-it-py", "markupsafe", "myst-nb", "pydata-sphinx-theme", "pygments-ipython-console", "pygments", "sphinx-autoapi", "sphinx-copybutton", "sphinx-design", "sphinx-tabs", "sphinx",] diff --git a/src/id3c/plans/flyscan_3idc.py b/src/id3c/plans/flyscan_3idc.py index 4532375..ca8b133 100644 --- a/src/id3c/plans/flyscan_3idc.py +++ b/src/id3c/plans/flyscan_3idc.py @@ -1072,6 +1072,8 @@ def build_flyscan_md( velocity_minimum, ad_file_name, ad_file_path, + ad_read_path_template="", + ad_write_path_template="", hdf_num_capture, hdf_flush_timeout_max, consumer_tick, @@ -1237,9 +1239,14 @@ def build_flyscan_md( "effective_v_min": v_min, # floor used "velocity_minimum_requested": velocity_minimum_md, # 0.0 if user passed None "velocity_minimum_was_default": velocity_minimum_was_default, - # File destination + # Detector data-file destination and the IOC->workstation path + # mapping (write = IOC side, read = workstation side), so a + # reader can locate the files from the master alone. Empty + # string when a template is unavailable. "ad_file_name": ad_file_name, "ad_file_path": ad_file_path, + "ad_read_path_template": ad_read_path_template or "", + "ad_write_path_template": ad_write_path_template or "", "hdf_num_capture": hdf_num_capture, # HDF plugin upper bound "hdf_flush_timeout_max": hdf_flush_timeout_max, # worst-case (s) "consumer_tick": consumer_tick, # monitor_loop wake-up tick (s) @@ -1292,6 +1299,16 @@ def _read_path_template(det): ) +def _write_path_template(det): + """IOC-side write path prefix for the detector's image files, or None. + + Sourced from ``det.hdf1.write_path_template``. + """ + return getattr(det.hdf1, "write_path_template", None) or getattr( + det.hdf1, "_write_path_template", None + ) + + def _check_ad_files_symlink(det, master_dir): """Warn if the image-files symlink is missing in ``master_dir``. @@ -1415,9 +1432,7 @@ def _external_link_target(det, ad_files_root=None): if ad_files_root is None: ad_files_root = ad_files_root_for(det) ioc_file = det.hdf1.full_file_name.get(use_monitor=False) - write_tmpl = getattr(det.hdf1, "write_path_template", None) or getattr( - det.hdf1, "_write_path_template", None - ) + write_tmpl = _write_path_template(det) if write_tmpl: prefix = write_tmpl if write_tmpl.endswith("/") else write_tmpl + "/" if ioc_file.startswith(prefix): @@ -2764,6 +2779,8 @@ def flyscan( velocity_minimum=velocity_minimum, ad_file_name=ad_file_name, ad_file_path=ad_file_path, + ad_read_path_template=_read_path_template(det) or "", + ad_write_path_template=_write_path_template(det) or "", hdf_num_capture=hdf_num_capture, hdf_flush_timeout_max=hdf_flush_timeout_max, consumer_tick=_consumer_tick, @@ -3021,112 +3038,19 @@ def update_master_file(): if df is not None: try: - flyscan_data_addr = "/entry/flyscan_data" - # frame_index = image_number - 1: IOC array_counter is - # 1-based, HDF5 dataset axes are 0-based. - image_number_arr = df["image_number"].to_numpy() - frame_index_arr = image_number_arr - 1 - n_frames_paired = int(len(df)) + from id3c.utils.flyscan_3idc_analysis import write_flyscan_data + # Expected total acquired-frame count (authoritative AD # file row count). The in-scan paired count can be # legitimately smaller (taxi-in / coast-out frames); # recorded as provenance for the reader. n_frames_expected = _expected_frame_count(external_file, run) - with h5py.File(external_file, "r") as src: - src_ds = src[external_addr + "/data"] - src_shape = src_ds.shape # (N, H, W) - src_dtype = src_ds.dtype - # VirtualLayout: out-shape (n_in_scan, H, W), - # each row sourced from src[frame_index[i], :, :]. - n_in_scan = len(frame_index_arr) - out_shape = (n_in_scan,) + tuple(src_shape[1:]) - layout = h5py.VirtualLayout(shape=out_shape, dtype=src_dtype) - vsrc = h5py.VirtualSource( - external_file, - name=external_addr + "/data", - shape=src_shape, - dtype=src_dtype, - ) - for out_i, src_i in enumerate(frame_index_arr): - layout[out_i] = vsrc[int(src_i)] - with h5py.File(master_file, "a") as root: - if flyscan_data_addr in root: - del root[flyscan_data_addr] - fs_grp = root.create_group(flyscan_data_addr) - fs_grp.attrs["NX_class"] = "NXdata" - fs_grp.attrs["signal"] = "data" - fs_grp.attrs["axes"] = ["position_start_acquire"] - - # Provenance: this group is sourced entirely from the - # authoritative AD HDF1 file. Recorded so a reader - # can confirm the source and frame counts from the - # file alone. - fs_grp.attrs["source"] = "ad_file" - fs_grp.attrs["source_description"] = ( - "Per-frame data read from the authoritative" - " area-detector HDF1 file (lossless, one row per" - " acquired frame)." - ) - fs_grp.attrs["n_frames_paired"] = n_frames_paired - if n_frames_expected is not None: - fs_grp.attrs["n_frames_expected"] = int(n_frames_expected) - - # Update the path to the NeXus default plot. - root["/entry"].attrs["default"] = "flyscan_data" - - # Primary signal: the in-scan image substack. - fs_grp.create_virtual_dataset("data", layout) - - # Plot axes (the position arrays). - fs_grp.create_dataset( - "position_start_acquire", - data=df["position_start_acquire"].to_numpy(), - ) - fs_grp.create_dataset( - "position_end_acquire", - data=df["position_end_acquire"].to_numpy(), - ) - fs_grp.create_dataset( - "position_end_period", - data=df["position_end_period"].to_numpy(), - ) - - # Subordinate per-frame correlation data. - ds_img = fs_grp.create_dataset( - "image_number", data=image_number_arr - ) - ds_img.attrs["description"] = ( - "IOC-side hdf1.array_counter value at frame" - " capture; 1-based per EPICS areaDetector" - " NDFileHDF5 plugin convention." - ) - ds_idx = fs_grp.create_dataset("frame_index", data=frame_index_arr) - ds_idx.attrs["target"] = "/entry/images/data" - ds_idx.attrs["description"] = ( - "0-based index into /entry/images/data along" - " its first axis; equal to image_number - 1." - " /entry/flyscan_data/data is already this" - " substack; use frame_index only to map back to" - " the full /entry/images/data stack:" - " images = f['/entry/images/data'];" - " idx = f['/entry/flyscan_data/frame_index'][:];" - " in_scan_images = images[idx, :, :]" - ) - fs_grp.create_dataset("timestamp", data=df["timestamp"].to_numpy()) - logger.info( - "flyscan._main: wrote %s (virtual 'data'" - " shape=%r dtype=%r from %s::%s, %d in-scan frame(s)," - " frame_index 0-based %d..%d) into %s and set" - " /entry@default='flyscan_data'", - flyscan_data_addr, - out_shape, - src_dtype, - external_file, - external_addr + "/data", - n_frames_paired, - int(frame_index_arr[0]), - int(frame_index_arr[-1]), + write_flyscan_data( master_file, + external_file, + df, + external_addr=external_addr, + n_frames_expected=n_frames_expected, ) except Exception as exc: logger.warning( diff --git a/src/id3c/tests/test_flyscan_repair.py b/src/id3c/tests/test_flyscan_repair.py new file mode 100644 index 0000000..207d48a --- /dev/null +++ b/src/id3c/tests/test_flyscan_repair.py @@ -0,0 +1,201 @@ +"""Tests for ``id3c.utils.flyscan_repair`` (the repair CLI). + +Synthetic master + area-detector HDF5 files; the catalog run is faked +and injected by monkeypatching ``_get_run``. No IOC, no live catalog. +""" + +from types import SimpleNamespace + +import h5py +import numpy as np +import pytest + +from id3c.utils import flyscan_repair +from id3c.utils.flyscan_3idc_analysis import EPICS_EPOCH_OFFSET_S + +UID = "abc123de-0000-1111-2222-333344445555" +UID_DSET = "/entry/instrument/NDAttributes/NDArrayUniqueId" +TS_DSET = "/entry/instrument/NDAttributes/NDArrayTimeStamp" + + +def _write_ad_file(path, n_frames=5, h=4, w=6, t_acquire=0.5, t0=1000.0): + """AD HDF1 file with an image stack + NDAttributes.""" + ad_unix_t = np.array([t0 + 1.0 + i for i in range(n_frames)]) + epics_t = ad_unix_t - EPICS_EPOCH_OFFSET_S + with h5py.File(path, "w") as f: + f.create_dataset("/entry/data/data", data=np.zeros((n_frames, h, w), "uint16")) + grp = f.create_group("/entry/instrument/NDAttributes") + grp.create_dataset("NDArrayTimeStamp", data=epics_t) + grp.create_dataset("NDArrayUniqueId", data=np.arange(n_frames, dtype=np.int32)) + return ad_unix_t + + +def _write_master(path, ad_file, *, use_link=True, ad_path=None, ad_name=None): + """Master with entry_identifier and either an external link or metadata.""" + with h5py.File(path, "w") as f: + f.create_dataset("/entry/entry_identifier", data=UID) + if use_link: + f["/entry/images"] = h5py.ExternalLink(str(ad_file), "/entry/data") + if ad_path is not None: + base = "/entry/instrument/bluesky/metadata/" + f.create_dataset(base + "ad_file_path", data=ad_path) + f.create_dataset(base + "ad_file_name", data=ad_name) + + +def _fake_run(ad_unix_t, *, p_start=0.0, p_end=10.0, t_acquire=0.5, t_period=1.0): + """Duck-typed run with metadata + motor monitor stream.""" + motor_t = np.linspace(1000.0, 1010.0, 101) + motor_pos = motor_t - 1000.0 # pos = 0..10 + md = { + "start": { + "p_start": p_start, + "p_end": p_end, + "flymotor_name": "m1", + "t_acquire": t_acquire, + "t_period": t_period, + } + } + ds = { + "time": SimpleNamespace(data=motor_t), + "m1": SimpleNamespace(data=motor_pos), + } + run = SimpleNamespace(metadata=md, m1_monitor=SimpleNamespace(read=lambda: ds)) + return run + + +@pytest.fixture +def _patch_run(monkeypatch): + """Inject a fake catalog run for any uid.""" + + def _factory(run): + monkeypatch.setattr(flyscan_repair, "_get_run", lambda uid: run) + + return _factory + + +def test_read_run_uid(tmp_path): + """The uid is read from /entry/entry_identifier.""" + ad = tmp_path / "ad.h5" + _write_ad_file(ad) + master = tmp_path / "m.hdf" + _write_master(master, ad) + assert flyscan_repair.read_run_uid(str(master)) == UID + + +def test_resolve_external_file_from_link(tmp_path): + """The external link target is resolved relative to the master dir.""" + ad = tmp_path / "sub" / "ad.h5" + ad.parent.mkdir() + _write_ad_file(ad) + master = tmp_path / "m.hdf" + # Link stored as a relative path from the master directory. + with h5py.File(master, "w") as f: + f.create_dataset("/entry/entry_identifier", data=UID) + f["/entry/images"] = h5py.ExternalLink("./sub/ad.h5", "/entry/data") + resolved = flyscan_repair.resolve_external_file(str(master)) + assert resolved == str(ad) + + +def test_resolve_external_file_from_metadata(tmp_path): + """With no link, the AD path is composed from start metadata.""" + master = tmp_path / "m.hdf" + _write_master( + master, + tmp_path / "unused.h5", + use_link=False, + ad_path="/data/run/", + ad_name="scan42", + ) + resolved = flyscan_repair.resolve_external_file(str(master)) + assert resolved == "/data/run/scan42_000001.h5" + + +def test_repair_writes_flyscan_data(tmp_path, _patch_run): + """A full repair writes /entry/flyscan_data with the paired frames.""" + ad = tmp_path / "ad.h5" + ad_unix_t = _write_ad_file(ad, n_frames=5) + master = tmp_path / "m.hdf" + _write_master(master, ad) + _patch_run(_fake_run(ad_unix_t)) + + summary = flyscan_repair.repair_master_file(str(master)) + + assert summary["written"] is True + assert summary["n_frames_paired"] > 0 + with h5py.File(master, "r") as f: + assert "/entry/flyscan_data" in f + grp = f["/entry/flyscan_data"] + assert grp.attrs["NX_class"] == "NXdata" + assert grp.attrs["source"] == "ad_file" + assert grp.attrs["n_frames_expected"] == 5 + assert f["/entry"].attrs["default"] == "flyscan_data" + + +def test_repair_is_idempotent(tmp_path, _patch_run): + """Running twice yields the same group (no crash on re-write).""" + ad = tmp_path / "ad.h5" + ad_unix_t = _write_ad_file(ad) + master = tmp_path / "m.hdf" + _write_master(master, ad) + _patch_run(_fake_run(ad_unix_t)) + + first = flyscan_repair.repair_master_file(str(master)) + second = flyscan_repair.repair_master_file(str(master)) + assert first["n_frames_paired"] == second["n_frames_paired"] + + +def test_dry_run_does_not_write(tmp_path, _patch_run): + """--dry-run reports but leaves the master unchanged.""" + ad = tmp_path / "ad.h5" + ad_unix_t = _write_ad_file(ad) + master = tmp_path / "m.hdf" + _write_master(master, ad) + _patch_run(_fake_run(ad_unix_t)) + + summary = flyscan_repair.repair_master_file(str(master), dry_run=True) + + assert summary["written"] is False + with h5py.File(master, "r") as f: + assert "/entry/flyscan_data" not in f + + +def test_missing_uid_raises(tmp_path): + """A master with no entry_identifier is an error.""" + master = tmp_path / "m.hdf" + with h5py.File(master, "w") as f: + f.create_group("/entry") + with pytest.raises(ValueError, match="run uid"): + flyscan_repair.repair_master_file(str(master)) + + +def test_unresolvable_ad_file_raises(tmp_path): + """A non-existent area-detector file is an error.""" + master = tmp_path / "m.hdf" + _write_master(master, tmp_path / "nope.h5") # link points at missing file + with pytest.raises(FileNotFoundError): + flyscan_repair.repair_master_file(str(master)) + + +def test_cli_main_success(tmp_path, _patch_run, capsys): + """The CLI returns 0 and prints a summary on success.""" + ad = tmp_path / "ad.h5" + ad_unix_t = _write_ad_file(ad) + master = tmp_path / "m.hdf" + _write_master(master, ad) + _patch_run(_fake_run(ad_unix_t)) + + rc = flyscan_repair.main([str(master)]) + assert rc == 0 + out = capsys.readouterr().out + assert "/entry/flyscan_data" in out + assert UID in out + + +def test_cli_main_failure_returns_1(tmp_path, capsys): + """The CLI returns 1 and reports the error on failure.""" + master = tmp_path / "m.hdf" + with h5py.File(master, "w") as f: + f.create_group("/entry") + rc = flyscan_repair.main([str(master)]) + assert rc == 1 + assert "FAILED" in capsys.readouterr().err diff --git a/src/id3c/tests/test_path_templates.py b/src/id3c/tests/test_path_templates.py new file mode 100644 index 0000000..5d0f810 --- /dev/null +++ b/src/id3c/tests/test_path_templates.py @@ -0,0 +1,42 @@ +"""Tests for the hdf1 path-template reader helpers. + +``_read_path_template`` / ``_write_path_template`` feed the +``ad_read_path_template`` / ``ad_write_path_template`` start-metadata +that lets a master file reconstruct its image-files symlink. +""" + +from types import SimpleNamespace + +from id3c.plans.flyscan_3idc import _read_path_template +from id3c.plans.flyscan_3idc import _write_path_template + + +def test_read_path_template_primary(): + """read_path_template is returned when present.""" + det = SimpleNamespace(hdf1=SimpleNamespace(read_path_template="/net/mount/")) + assert _read_path_template(det) == "/net/mount/" + + +def test_write_path_template_primary(): + """write_path_template is returned when present.""" + det = SimpleNamespace(hdf1=SimpleNamespace(write_path_template="/ioc/path/")) + assert _write_path_template(det) == "/ioc/path/" + + +def test_read_falls_back_to_underscore_attr(): + """The _read_path_template attribute is the documented fallback.""" + det = SimpleNamespace(hdf1=SimpleNamespace(_read_path_template="/net/mount/")) + assert _read_path_template(det) == "/net/mount/" + + +def test_write_falls_back_to_underscore_attr(): + """The _write_path_template attribute is the documented fallback.""" + det = SimpleNamespace(hdf1=SimpleNamespace(_write_path_template="/ioc/path/")) + assert _write_path_template(det) == "/ioc/path/" + + +def test_returns_none_when_absent(): + """Missing templates return None (stamped as '' in metadata).""" + det = SimpleNamespace(hdf1=SimpleNamespace()) + assert _read_path_template(det) is None + assert _write_path_template(det) is None diff --git a/src/id3c/tests/test_write_flyscan_data.py b/src/id3c/tests/test_write_flyscan_data.py new file mode 100644 index 0000000..6bb47c1 --- /dev/null +++ b/src/id3c/tests/test_write_flyscan_data.py @@ -0,0 +1,117 @@ +"""Tests for ``id3c.utils.flyscan_3idc_analysis.write_flyscan_data``. + +Verifies the on-disk ``/entry/flyscan_data`` layout, the virtual +dataset selection, the provenance attributes, and idempotency. +Synthetic AD file + a hand-built paired DataFrame; no IOC, no catalog. +""" + +import h5py +import numpy as np +import pandas as pd + +from id3c.utils.flyscan_3idc_analysis import write_flyscan_data + + +def _write_ad_file(path, n_frames=6, h=3, w=4): + """AD HDF1 file whose /entry/data/data[i] is filled with value i.""" + data = np.stack([np.full((h, w), i, dtype="uint16") for i in range(n_frames)]) + with h5py.File(path, "w") as f: + f.create_dataset("/entry/data/data", data=data) + return data + + +def _paired_df(image_numbers): + """Minimal pairing DataFrame with the required columns.""" + n = len(image_numbers) + return pd.DataFrame( + { + "image_number": np.asarray(image_numbers, dtype=np.int64), + "timestamp": np.arange(n, dtype=float), + "position_start_acquire": np.arange(n, dtype=float), + "position_end_acquire": np.arange(n, dtype=float) + 0.1, + "position_end_period": np.arange(n, dtype=float) + 0.2, + } + ) + + +def test_writes_expected_layout(tmp_path): + """All datasets and group attributes are present and correct.""" + ad = tmp_path / "ad.h5" + _write_ad_file(ad, n_frames=6) + master = tmp_path / "m.hdf" + with h5py.File(master, "w") as f: + f.create_group("/entry") + # In-scan frames are image_number 2,3,4 (1-based) -> idx 1,2,3. + df = _paired_df([2, 3, 4]) + + summary = write_flyscan_data(str(master), str(ad), df, n_frames_expected=6) + + assert summary["n_frames_paired"] == 3 + with h5py.File(master, "r") as f: + grp = f["/entry/flyscan_data"] + assert grp.attrs["NX_class"] == "NXdata" + assert grp.attrs["signal"] == "data" + assert grp.attrs["source"] == "ad_file" + assert grp.attrs["n_frames_paired"] == 3 + assert grp.attrs["n_frames_expected"] == 6 + assert f["/entry"].attrs["default"] == "flyscan_data" + for name in ( + "data", + "position_start_acquire", + "position_end_acquire", + "position_end_period", + "image_number", + "frame_index", + "timestamp", + ): + assert name in grp, name + assert list(grp["frame_index"][()]) == [1, 2, 3] + assert list(grp["image_number"][()]) == [2, 3, 4] + + +def test_virtual_dataset_selects_correct_frames(tmp_path): + """The virtual 'data' maps frame_index into the source stack.""" + ad = tmp_path / "ad.h5" + _write_ad_file(ad, n_frames=6) + master = tmp_path / "m.hdf" + with h5py.File(master, "w") as f: + f.create_group("/entry") + df = _paired_df([2, 4, 6]) # idx 1, 3, 5 + + write_flyscan_data(str(master), str(ad), df) + + with h5py.File(master, "r") as f: + data = f["/entry/flyscan_data/data"][()] + # Each source frame i is filled with value i; expect 1, 3, 5. + assert [int(plane.flat[0]) for plane in data] == [1, 3, 5] + + +def test_idempotent_rewrite(tmp_path): + """Writing twice replaces the group without error.""" + ad = tmp_path / "ad.h5" + _write_ad_file(ad, n_frames=6) + master = tmp_path / "m.hdf" + with h5py.File(master, "w") as f: + f.create_group("/entry") + df = _paired_df([2, 3]) + + write_flyscan_data(str(master), str(ad), df) + write_flyscan_data(str(master), str(ad), df) # must not raise + + with h5py.File(master, "r") as f: + assert f["/entry/flyscan_data/data"].shape[0] == 2 + + +def test_n_frames_expected_omitted_when_none(tmp_path): + """No n_frames_expected attribute when the argument is None.""" + ad = tmp_path / "ad.h5" + _write_ad_file(ad, n_frames=4) + master = tmp_path / "m.hdf" + with h5py.File(master, "w") as f: + f.create_group("/entry") + df = _paired_df([1, 2]) + + write_flyscan_data(str(master), str(ad), df, n_frames_expected=None) + + with h5py.File(master, "r") as f: + assert "n_frames_expected" not in f["/entry/flyscan_data"].attrs diff --git a/src/id3c/utils/flyscan_3idc_analysis.py b/src/id3c/utils/flyscan_3idc_analysis.py index 24febdd..f5189ea 100644 --- a/src/id3c/utils/flyscan_3idc_analysis.py +++ b/src/id3c/utils/flyscan_3idc_analysis.py @@ -1053,3 +1053,151 @@ def hdf_timestamp_semantic_diagnostic(run) -> dict: "noisy_data": noisy_data, "indecisive": indecisive, } + + +def write_flyscan_data( + master_file, + external_file, + df, + *, + external_addr="/entry/data", + n_frames_expected=None, +): + """Write the ``/entry/flyscan_data`` group into the NeXus master file. + + This is the single primary-product group: an ``NXdata`` holding the + in-scan image substack (an ``h5py.VirtualLayout`` into the external + area-detector file, no bytes copied) plus the per-frame correlation + data, all from the authoritative AD HDF1 file. + + Both the live flyscan plan and the offline repair tool call this so + the on-disk layout is identical regardless of when it is written. + Any pre-existing ``/entry/flyscan_data`` is replaced (idempotent). + + Parameters + ---------- + master_file : str + Path to the NeXus master HDF5 file (opened for append). + external_file : str + Path to the area-detector HDF1 file, resolvable from the + master file's directory (i.e. through the image-files symlink). + df : pandas.DataFrame + Output of ``pair_frames_to_positions_from_ad_file``; one row + per in-scan frame with ``image_number``, ``timestamp``, and the + three ``position_*`` columns. + external_addr : str + Group inside ``external_file`` holding ``data`` (the image + stack). Defaults to ``/entry/data``. + n_frames_expected : int or None + Total acquired-frame count, recorded as provenance. ``None`` + omits the attribute. + + Returns + ------- + dict + Summary: ``n_frames_paired``, ``out_shape``, ``src_dtype``. + """ + import h5py + + flyscan_data_addr = "/entry/flyscan_data" + # frame_index = image_number - 1: IOC array_counter is 1-based, + # HDF5 dataset axes are 0-based. + image_number_arr = df["image_number"].to_numpy() + frame_index_arr = image_number_arr - 1 + n_frames_paired = int(len(df)) + + with h5py.File(external_file, "r") as src: + src_ds = src[external_addr + "/data"] + src_shape = src_ds.shape # (N, H, W) + src_dtype = src_ds.dtype + # VirtualLayout: out-shape (n_in_scan, H, W), each row sourced + # from src[frame_index[i], :, :]. + n_in_scan = len(frame_index_arr) + out_shape = (n_in_scan,) + tuple(src_shape[1:]) + layout = h5py.VirtualLayout(shape=out_shape, dtype=src_dtype) + vsrc = h5py.VirtualSource( + external_file, + name=external_addr + "/data", + shape=src_shape, + dtype=src_dtype, + ) + for out_i, src_i in enumerate(frame_index_arr): + layout[out_i] = vsrc[int(src_i)] + + with h5py.File(master_file, "a") as root: + if flyscan_data_addr in root: + del root[flyscan_data_addr] + fs_grp = root.create_group(flyscan_data_addr) + fs_grp.attrs["NX_class"] = "NXdata" + fs_grp.attrs["signal"] = "data" + fs_grp.attrs["axes"] = ["position_start_acquire"] + + # Provenance: this group is sourced entirely from the + # authoritative AD HDF1 file. + fs_grp.attrs["source"] = "ad_file" + fs_grp.attrs["source_description"] = ( + "Per-frame data read from the authoritative area-detector" + " HDF1 file (lossless, one row per acquired frame)." + ) + fs_grp.attrs["n_frames_paired"] = n_frames_paired + if n_frames_expected is not None: + fs_grp.attrs["n_frames_expected"] = int(n_frames_expected) + + # Update the path to the NeXus default plot. + root["/entry"].attrs["default"] = "flyscan_data" + + # Primary signal: the in-scan image substack. + fs_grp.create_virtual_dataset("data", layout) + + # Plot axes (the position arrays). + fs_grp.create_dataset( + "position_start_acquire", + data=df["position_start_acquire"].to_numpy(), + ) + fs_grp.create_dataset( + "position_end_acquire", + data=df["position_end_acquire"].to_numpy(), + ) + fs_grp.create_dataset( + "position_end_period", + data=df["position_end_period"].to_numpy(), + ) + + # Subordinate per-frame correlation data. + ds_img = fs_grp.create_dataset("image_number", data=image_number_arr) + ds_img.attrs["description"] = ( + "IOC-side hdf1.array_counter value at frame capture; 1-based" + " per EPICS areaDetector NDFileHDF5 plugin convention." + ) + ds_idx = fs_grp.create_dataset("frame_index", data=frame_index_arr) + ds_idx.attrs["target"] = "/entry/images/data" + ds_idx.attrs["description"] = ( + "0-based index into /entry/images/data along its first axis;" + " equal to image_number - 1. /entry/flyscan_data/data is" + " already this substack; use frame_index only to map back to" + " the full /entry/images/data stack:" + " images = f['/entry/images/data'];" + " idx = f['/entry/flyscan_data/frame_index'][:];" + " in_scan_images = images[idx, :, :]" + ) + fs_grp.create_dataset("timestamp", data=df["timestamp"].to_numpy()) + + logger.info( + "write_flyscan_data: wrote %s (virtual 'data' shape=%r dtype=%r" + " from %s::%s, %d in-scan frame(s), frame_index 0-based %d..%d)" + " into %s and set /entry@default='flyscan_data'", + flyscan_data_addr, + out_shape, + src_dtype, + external_file, + external_addr + "/data", + n_frames_paired, + int(frame_index_arr[0]), + int(frame_index_arr[-1]), + master_file, + ) + return { + "n_frames_paired": n_frames_paired, + "out_shape": out_shape, + "src_dtype": src_dtype, + } diff --git a/src/id3c/utils/flyscan_repair.py b/src/id3c/utils/flyscan_repair.py new file mode 100644 index 0000000..a602df9 --- /dev/null +++ b/src/id3c/utils/flyscan_repair.py @@ -0,0 +1,246 @@ +"""Repair tool: add ``/entry/flyscan_data`` to an existing master file. + +A flyscan master file is missing ``/entry/flyscan_data`` when the +area-detector file was not reachable at run end (most commonly the +image-files symlink next to the master was missing or mis-shaped). +Once the symlink is fixed, this tool recomputes the per-frame pairing +and writes the group, producing the same on-disk layout the live plan +would have written. + +Console script (see ``pyproject.toml``):: + + id3c-flyscan-repair MASTER.hdf [--external-file PATH] [--dry-run] + +The run is identified by the uid stored in the master file at +``/entry/entry_identifier``; the catalog is read from +``id3c.startup.cat``. The area-detector file is located from the +master's existing ``/entry/images`` external link, or composed from +the ``ad_file_path`` / ``ad_file_name`` start-document metadata. +""" + +from __future__ import annotations + +import argparse +import logging +import os +import sys + +logger = logging.getLogger(__name__) + +ENTRY_IDENTIFIER = "/entry/entry_identifier" +IMAGES_ADDR = "/entry/images" +AD_FILE_NUMBER_SUFFIX = "_000001.h5" + + +def _read_str(h5obj, addr): + """Return a string dataset/attr value at ``addr`` or None.""" + if addr not in h5obj: + return None + val = h5obj[addr][()] + if isinstance(val, bytes): + return val.decode() + return str(val) + + +def read_run_uid(master_file): + """Return the run uid stored in the master file, or None.""" + import h5py + + with h5py.File(master_file, "r") as f: + return _read_str(f, ENTRY_IDENTIFIER) + + +def resolve_external_file(master_file): + """Return the area-detector file path resolvable from the master. + + Prefers the existing ``/entry/images`` external-link target + (resolved relative to the master's directory through the + image-files symlink). Falls back to composing the IOC path from + the ``ad_file_path`` / ``ad_file_name`` start metadata. Returns + ``None`` if neither is available. + """ + import h5py + + master_dir = os.path.dirname(os.path.abspath(master_file)) + with h5py.File(master_file, "r") as f: + link = f.get(IMAGES_ADDR, getlink=True) + if isinstance(link, h5py.ExternalLink): + return os.path.normpath(os.path.join(master_dir, link.filename)) + + base = "/entry/instrument/bluesky/metadata/" + ad_path = _read_str(f, base + "ad_file_path") + ad_name = _read_str(f, base + "ad_file_name") + + if ad_path and ad_name: + return os.path.join(ad_path, ad_name + AD_FILE_NUMBER_SUFFIX) + return None + + +def _get_run(uid): + """Return the catalog run for ``uid`` (imported lazily).""" + from id3c.startup import cat + + return cat[uid] + + +def repair_master_file(master_file, *, external_file=None, dry_run=False): + """Recompute and write ``/entry/flyscan_data`` into ``master_file``. + + Returns a summary dict. Raises on unrecoverable conditions + (no uid, no AD file, AD file not openable, 0 paired frames) so the + CLI can report a non-zero exit. + """ + import h5py + + from id3c.utils.flyscan_3idc_analysis import pair_frames_to_positions_from_ad_file + + uid = read_run_uid(master_file) + if not uid: + raise ValueError( + f"{master_file!r} has no {ENTRY_IDENTIFIER} (run uid); cannot" + " locate the catalog run." + ) + + if external_file is None: + external_file = resolve_external_file(master_file) + if not external_file: + raise ValueError( + f"could not determine the area-detector file for {master_file!r};" + " pass --external-file explicitly." + ) + + if not os.path.exists(external_file): + raise FileNotFoundError( + f"area-detector file {external_file!r} does not exist or does" + " not resolve (check the image-files symlink next to the" + " master file)." + ) + try: + with h5py.File(external_file, "r"): + pass + except OSError as exc: + raise OSError( + f"area-detector file {external_file!r} is not openable: {exc!r}" + ) from exc + + run = _get_run(uid) + df = pair_frames_to_positions_from_ad_file(run, external_file) + n_paired = int(len(df)) + if n_paired == 0: + raise ValueError( + f"pairing produced 0 in-scan frames for uid={uid!r}; nothing to write." + ) + + if dry_run: + logger.info( + "flyscan-repair: DRY RUN — would write /entry/flyscan_data" + " (%d in-scan frame(s)) into %s from %s", + n_paired, + master_file, + external_file, + ) + return { + "uid": uid, + "external_file": external_file, + "n_frames_paired": n_paired, + "written": False, + } + + # Write only the function's own provenance (n_frames_expected from + # the AD file frame count) without depending on the plan module. + n_frames_expected = _ad_frame_count(external_file) + from id3c.utils.flyscan_3idc_analysis import write_flyscan_data + + summary = write_flyscan_data( + master_file, + external_file, + df, + n_frames_expected=n_frames_expected, + ) + logger.info( + "flyscan-repair: wrote /entry/flyscan_data (%d in-scan frame(s)) into %s", + summary["n_frames_paired"], + master_file, + ) + return { + "uid": uid, + "external_file": external_file, + "n_frames_paired": summary["n_frames_paired"], + "written": True, + } + + +def _ad_frame_count( + external_file, + unique_id_dset="/entry/instrument/NDAttributes/NDArrayUniqueId", +): + """Total acquired-frame count from the AD file, or None.""" + import h5py + + try: + with h5py.File(external_file, "r") as f: + if unique_id_dset in f: + return int(f[unique_id_dset].shape[0]) + except Exception as exc: + logger.debug("_ad_frame_count: %r", exc) + return None + + +def main(argv=None): + """Console-script entry point.""" + parser = argparse.ArgumentParser( + prog="id3c-flyscan-repair", + description=( + "Add the missing /entry/flyscan_data group to a flyscan" + " NeXus master file by recomputing the per-frame pairing" + " from the authoritative area-detector file." + ), + ) + parser.add_argument("master_file", help="path to the NeXus master HDF5 file") + parser.add_argument( + "--external-file", + default=None, + help=( + "explicit path to the area-detector HDF1 file (overrides the" + " path resolved from the master file)" + ), + ) + parser.add_argument( + "--dry-run", + action="store_true", + help="report what would be done without modifying the master file", + ) + parser.add_argument( + "-v", + "--verbose", + action="store_true", + help="enable INFO-level logging", + ) + args = parser.parse_args(argv) + + logging.basicConfig( + level=logging.INFO if args.verbose else logging.WARNING, + format="%(levelname)s: %(message)s", + ) + + try: + summary = repair_master_file( + args.master_file, + external_file=args.external_file, + dry_run=args.dry_run, + ) + except Exception as exc: + print(f"flyscan-repair: FAILED: {exc}", file=sys.stderr) + return 1 + + action = "would write" if args.dry_run else "wrote" + print( + f"flyscan-repair: {action} /entry/flyscan_data" + f" ({summary['n_frames_paired']} in-scan frame(s)) for uid" + f" {summary['uid']} using {summary['external_file']}" + ) + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) From c00ec781cc44cf829a37a26264056f3a45afb4b5 Mon Sep 17 00:00:00 2001 From: Pete Jemian Date: Wed, 24 Jun 2026 12:25:20 -0500 Subject: [PATCH 7/8] DOC: strip historical narration from flyscan comments/docstrings Remove dates, session/notes/strategy-doc references, Phase-N decision labels, and 'empirically/formerly/revised' framing from source comments and docstrings, keeping the present-tense technical facts. Per the AGENTS.md convention; history lives in issues/PRs/commits. --- src/id3c/plans/flyscan_3idc.py | 305 ++++++++++-------------- src/id3c/utils/flyscan_3idc_analysis.py | 38 ++- 2 files changed, 140 insertions(+), 203 deletions(-) diff --git a/src/id3c/plans/flyscan_3idc.py b/src/id3c/plans/flyscan_3idc.py index ca8b133..2181f1c 100644 --- a/src/id3c/plans/flyscan_3idc.py +++ b/src/id3c/plans/flyscan_3idc.py @@ -103,8 +103,8 @@ def ad_files_root_for(det): """Soft cap on ``num_images * acquire_period`` (~3.5 days). The Eiger refuses to arm when the implied total acquisition -duration exceeds ~600_000 s (probed 2026-06-16). Stay well under -that. See ``effective_num_images``. +duration exceeds ~600_000 s. Stay well under that. See +``effective_num_images``. """ @@ -143,7 +143,7 @@ def effective_num_images(t_period: float) -> int: # Module-level tunable timing constants. # # These are the wake-up ticks for plan-side loops that wait on a -# status-object flag (Phase 3 refactor). CA monitor callbacks update +# status-object flag. CA monitor callbacks update # the flags asynchronously on the pyepics dispatch thread; the plan # wakes up at these intervals to check the flag and decide whether to # proceed. @@ -171,7 +171,7 @@ def effective_num_images(t_period: float) -> int: # At typical scan rates (10 Hz frames, 20 ms consumer tick), the # queue stays nearly empty. This bound exists to detect a # producer/consumer mismatch — overflow is logged once as a WARNING -# and the entry is dropped (per Phase 3 decision 3.3b). Increase +# and the entry is dropped. Increase # here if higher-rate scans are ever attempted. _FRAME_QUEUE_SIZE = 64 @@ -242,21 +242,11 @@ def read_motor_field(motor, suffix, timeout=1.0): within ``timeout`` seconds or any other read error occurs. Uses ``epics.caget`` directly rather than constructing a throwaway - ``EpicsSignal``. Rationale: - - * ``caget`` runs on the calling thread's CA context and does no - asynchronous metadata fetching. An earlier ``EpicsSignal``-based - implementation triggered a SIGSEGV in pyepics' ``util3`` dispatch - thread on ``gp:m1.VBAS``: the ``EpicsSignal`` was created, read, - then ``destroy()``ed, but pyepics had queued a deferred metadata - callback (``get_all_metadata_callback``) that fired *after* the - underlying CA channel had been torn down — at first a recoverable - ``RuntimeError: Expected CA context is unset`` (2026-06-04 - 12:46:38 session), then a segfault (2026-06-04 13:10 session). - * ``caget`` also sidesteps the oregistry-pollution concern (the - previous implementation needed a ``_suspended_auto_register`` - context manager to keep the throwaway signal out of the global - registry). + ``EpicsSignal``. ``caget`` runs on the calling thread's CA context + and does no asynchronous metadata fetching; a throwaway + ``EpicsSignal`` can segfault when pyepics fires a deferred metadata + callback after the signal's CA channel is torn down, and would also + need suppression to avoid polluting the global oregistry. """ pv = motor.prefix + suffix try: @@ -528,22 +518,19 @@ def configure_adsimdet( # 5. Capture sizing. # # In 'Capture' mode, num_capture is an *upper bound*: the plugin - # stops capturing when this count is reached, but the on-disk - # dataset is sized to whatever number of frames actually got - # captured (verified by experiment: num_capture=5 with only 3 - # frames arriving produced a (3, H, W) dataset, not (5, H, W)). + # stops capturing when this count is reached, and the on-disk + # dataset is sized to the number of frames actually captured. # # CAUTION: num_capture cannot be made arbitrarily large. The IOC's # NDFileHDF5 plugin computes byte counts as C int arithmetic during # dataset pre-allocation; values around 1e9 with Float64 1024x1024 # frames overflow, producing a file whose num_captured counter - # advances but whose /entry/data/data dataset is never written - # (verified empirically). num_capture <= ~1e6 is known safe for - # typical frame sizes. + # advances but whose /entry/data/data dataset is never written. + # Keep num_capture <= ~1e6 for typical frame sizes. # - # We choose num_capture sized for the expected count times 1.5 - # plus 20, which absorbs (takeoff & landing) leading edges, post-stop - # tail frames, and timing jitter for any sensible scan size. + # num_capture is sized for the expected count times 1.5 plus 20, + # which absorbs takeoff/landing leading edges, post-stop tail + # frames, and timing jitter for any sensible scan size. expected_frames = int(capture_duration / acquire_period) if num_capture is None: num_capture = int(expected_frames * 1.5) + 20 @@ -571,8 +558,8 @@ def configure_adsimdet( det.cam.num_images.put(UNLIMITED_FRAMES) # 7. Arm the HDF plugin and *wait* for it to reach the 'Capturing' - # state. This was a race in earlier tests: cam frames arrived - # before capture was ready, and were silently dropped. + # state. Without the wait, cam frames that arrive before capture + # is ready are silently dropped. # # ``capture`` is an EpicsSignalWithRBV; ``set(1)`` runs the generic # Signal.set() path which puts and then polls Capture_RBV until it @@ -636,9 +623,8 @@ def configure_adsimdet( # # num_queued_arrays is an RBV-only PV (no .set() to lean on); # use the _wait_for helper. Timeout is a warning, not a - # raise, matching the original behavior. int() cast in the - # predicate guards against pyepics returning the value as a - # string for certain record types. + # raise. int() cast in the predicate guards against pyepics + # returning the value as a string for certain record types. try: _wait_for( det.hdf1.num_queued_arrays, @@ -659,9 +645,9 @@ def configure_adsimdet( # 12. Explicitly flush the file to disk. WriteFile=1 forces # the plugin to write out whatever it has captured. This # is required when stopping capture before num_capture is - # reached: empirically, neither Capture=0 alone nor - # auto_save=Yes is sufficient in that case (the file ends - # up with the NeXus skeleton but no image dataset). + # reached: neither Capture=0 alone nor auto_save=Yes flushes + # in that case (the file ends up with the NeXus skeleton but + # no image dataset). captured = det.hdf1.num_captured.get(use_monitor=False) if captured > 0: logger.info( @@ -974,10 +960,7 @@ def validate_flyscan_inputs( f" .VELO unreadable or non-positive (got {v_velo!r})." ) - # Effective ceiling: per user spec, .VELO is the cap (independent - # of .VMAX). The TODO-formula MIN(VELO, MAX(VMAX, VELO)) reduces - # to VELO in all cases — both branches of MAX yield a value >= VELO, - # so MIN with VELO gives VELO. + # Effective ceiling: .VELO is the cap, independent of .VMAX. v_max = float(v_velo) # Effective floor: start with .VBAS (None/0 sentinel => no IOC @@ -1091,17 +1074,15 @@ def build_flyscan_md( appear without the underscore in the metadata for readability. ``plan_name`` is recorded explicitly here (defaulting to - ``"flyscan"`` in the calling ``flyscan(...)`` plan). We cannot - rely on bluesky's ``RunEngine`` start-doc auto-derivation - (``getattr(self._plan, "__name__", "")``) because the + ``"flyscan"`` in the calling ``flyscan(...)`` plan). Bluesky's + ``RunEngine`` start-doc auto-derivation + (``getattr(self._plan, "__name__", "")``) cannot be used: the ``@bluesky.utils.plan`` decorator wraps the generator in a - ``Plan`` class instance with no ``__name__`` attribute, so the - auto-derived value is the empty string. Empirically confirmed - on 2026-06-10 against bluesky's current ``Plan`` implementation - (``__slots__ = ("_iter", "_stack")`` — no ``__name__`` slot). - Wrapper plans should pass their own name via ``flyscan(..., - plan_name="my_wrapper_name", ...)`` so the run's provenance - reflects the wrapper, not the inner flyscan call. + ``Plan`` instance with no ``__name__`` attribute, so the + auto-derived value is the empty string. Wrapper plans should + pass their own name via ``flyscan(..., plan_name="my_wrapper", + ...)`` so the run's provenance reflects the wrapper, not the + inner flyscan call. ``None``-substitution policy ---------------------------- @@ -1111,14 +1092,13 @@ def build_flyscan_md( ``apstools.callbacks.nexus_writer.NXWriter.write_metadata`` runs ``h5py.Group.create_dataset(k, data=v)`` on each item, and ``data=None`` raises ``TypeError: One of data, shape or dtype - must be specified`` (verified on the gp:m1 + adsimdet IOC at - 11:05:25, 2026-06-10 — see ``flyscan_3idc_notes.md``). + must be specified``. - For each input that may be ``None``, we substitute the value the + For each input that may be ``None``, substitute the value the validator / plan effectively used in its place, and record a sibling boolean so the provenance (real value vs substituted - default) is recoverable from metadata. Symmetric with the - existing ``motor_accl_was_default`` pattern: + default) is recoverable from metadata, matching the + ``motor_accl_was_default`` pattern: +------------------------------+------------------+--------------------------------+ | metadata key | None substitute | companion boolean | @@ -1181,10 +1161,8 @@ def build_flyscan_md( per-frame motor-position interpolation at the three meaningful per-period phases (start_acquire, end_acquire, end_period). Defaults to ``-t_acquire`` in the calling - ``flyscan(...)`` plan (the Phase 0 verdict for the - gp:m1 + adsimdet IOC; see ``flyscan_3idc_notes.md``). Pass - a per-call override if your IOC's HDF plugin timestamps - counter events at a different phase. + ``flyscan(...)`` plan. Pass a per-call override if your IOC's + HDF plugin timestamps counter events at a different phase. """ # noqa E501 # Substitute h5py-serialisable defaults for any None values, and # record companion booleans preserving the provenance. See the @@ -1254,10 +1232,8 @@ def build_flyscan_md( # the corresponding frame's start-of-acquire moment. Used by # flyscan_3idc_analysis.pair_frames_to_positions to compute # per-frame start_acquire / end_acquire / end_period - # timestamps. See flyscan_3idc_notes.md "Phase 0 verdict" - # for how the default (-t_acquire on gp:m1 + adsimdet) was - # determined. Per-call overridable via flyscan(..., - # hdf_t_phase_offset=...). + # timestamps. Default -t_acquire; per-call overridable via + # flyscan(..., hdf_t_phase_offset=...). "hdf_t_phase_offset": hdf_t_phase_offset, # Names of any extra readables passed to flyscan(detectors=). "detector_names": list(detector_names), @@ -1536,8 +1512,8 @@ def _expected_frame_count( # Cam state values (DetectorState_RBV enum) that indicate the cam -# failed to arm or is otherwise unusable. Empirically verified on -# the Eiger 2026-06-16: failed arm transitions to 'Error' within ~2 ms. +# failed to arm or is otherwise unusable. On the Eiger a failed arm +# transitions to 'Error' within ~2 ms. _CAM_ERROR_STATES = {"Error", "Aborted", "Aborting", "Disconnected"} @@ -1600,9 +1576,8 @@ def motor_is_moving(motor): """True iff the motor is currently moving, checked via DMOV (no cache). Reads ``motor_done_move`` (the motor record's ``.DMOV`` field) with - ``use_monitor=False`` to bypass the pyepics monitor cache; this - matches the rest of this module's "bypass cache for timing-critical - reads" discipline (see strategy-doc Phase 0.1 for background). + ``use_monitor=False`` to bypass the pyepics monitor cache, matching + this module's "bypass cache for timing-critical reads" discipline. Prefer this function over ``motor.moving`` (the EpicsMotor property), which depending on ophyd version may read ``MOVN`` rather than @@ -1611,8 +1586,7 @@ def motor_is_moving(motor): ``DMOV`` only goes to 1 when both phases are complete. Used by ``_cleanup`` to decide whether to issue ``bps.stop(flymotor)`` - (skipped if the motor is already idle, per Phase 2 decision 2.5 / - Phase 3 decision 3.12). + (skipped if the motor is already idle). """ return motor.motor_done_move.get(use_monitor=False, as_string=False) != 1 @@ -1828,7 +1802,7 @@ def wait_for_acquire_drained(det, poll=0.001, timeout=10.0): underlying status updates happen on the CA monitor thread; ``poll`` only controls how soon the plan notices. A future-proofing alternative would be ``bps.wait_for([asyncio_future])`` (would tie - the plan to the RunEngine's asyncio loop; see strategy doc Tier 4). + the plan to the RunEngine's asyncio loop). """ has_busy = _has_component(det.cam, "acquire_busy") has_hdf_queue = _has_component(det, "hdf1") and _has_component( @@ -1863,10 +1837,9 @@ def wait_for_acquire_drained(det, poll=0.001, timeout=10.0): ) ) if has_hdf_queue: - # HDF-drain predicate preserves the original race-window - # protection: queue empty AND all slots free (i.e. no frame - # is currently being written). The two corroboration reads - # use use_monitor=False so the predicate sees consistent + # HDF-drain predicate: queue empty AND all slots free (i.e. no + # frame is currently being written). The two corroboration + # reads use use_monitor=False so the predicate sees consistent # post-update values rather than a stale cache. hdf = det.hdf1 sub_statuses.append( @@ -1936,7 +1909,7 @@ def monitor_loop( ): """Plan stub: emit one primary-stream event per HDF frame written. - Producer/consumer design (Phase 3.C refactor): + Producer/consumer design: * **Producer:** a CA monitor callback on ``det.hdf1.num_captured`` pushes ``(timestamp, new_value)`` onto a small bounded @@ -1948,10 +1921,10 @@ def monitor_loop( thread; ``yield from bps.sleep(tick)`` keeps the RunEngine in control of pause/abort. - Per Phase 0.2 / Phase 0e: the primary stream is a progress - indicator. Pairing of detector frames to flymotor positions - happens downstream via the IOC-timestamped monitor streams set - up by ``@bpp.monitor_during_decorator``; the per-frame + The primary stream is a progress indicator. Pairing of detector + frames to flymotor positions happens downstream via the + IOC-timestamped monitor streams set up by + ``@bpp.monitor_during_decorator``; the per-frame ``bps.read(det)`` + ``bps.read(flymotor)`` here is a snapshot, not the system of record, and uses the cached monitor values (no extra CA traffic). @@ -1978,12 +1951,10 @@ def monitor_loop( pass a ``motor_stopped_flag`` (see below) to discriminate "stopped on purpose" from "stopped because something else broke." - This check happens on each consumer tick (per Phase 3 decision - 3.6 — overshoot is dominated by the motor record's ~10 Hz - update rate, not by the tick). + This check happens on each consumer tick; overshoot is dominated + by the motor record's ~10 Hz update rate, not by the tick. - Exit: ``exit_when.done``. Per Phase 3 decisions 3.8 / 3.B, the - caller constructs ``exit_when`` as + Exit: ``exit_when.done``. The caller constructs ``exit_when`` as ``AndStatus(cam_stopped_status, drain_status)`` so the loop exits only when the cam has been stopped AND every in-flight frame has been flushed by the HDF plugin. @@ -1994,9 +1965,9 @@ def monitor_loop( completes the status as failed if no frame arrives in time; the consumer checks ``watchdog.done and not watchdog.success`` per tick and raises ``RuntimeError`` annotated with the HDF - plugin's ``WriteStatus`` and ``WriteMessage``. Per Phase 3 - decision 3.11, the raise lets the RunEngine send STOP to all - in-motion movables (including ``flymotor``), which is exactly + plugin's ``WriteStatus`` and ``WriteMessage``. The raise lets + the RunEngine send STOP to all in-motion movables (including + ``flymotor``), which is exactly what we want when something is wrong with the IOC/cam/HDF chain. @@ -2041,7 +2012,7 @@ def _on_num_captured(*, value, timestamp, **kwargs): Pushes ``(timestamp, value)`` onto ``frame_queue``. Drops the entry and logs a single WARNING if the queue is full - (per Phase 3 decision 3.3b — never block the CA thread). + (never block the CA thread). """ try: frame_queue.put_nowait((timestamp, int(value))) @@ -2057,7 +2028,7 @@ def _on_num_captured(*, value, timestamp, **kwargs): _FRAME_QUEUE_SIZE, ) - # --- Inner helpers (per Phase 3 decision 3.1: named for clarity) + # --- Inner helpers (named for clarity) def _emit_pending_frames(last_captured, *, acquire_stopped): """Drain the queue and emit one primary event per new frame. @@ -2138,8 +2109,7 @@ def _check_p_end_crossing(acquire_stopped, last_captured): """ if acquire_stopped: return acquire_stopped - # Bypass cache for the position read — pairing timing matters - # here (Phase 0.1 decision). + # Bypass cache for the position read — pairing timing matters. if flymotor.user_readback.get(use_monitor=False) >= p_end: logger.info( "monitor_loop: motor crossed p_end (%g); stopping acquire" @@ -2164,16 +2134,15 @@ def _check_watchdog(): Returns nothing; raises RuntimeError on watchdog trip. Watchdog timeout is signalled by ophyd's StatusBase as ``done=True, success=False`` after the timeout elapses - without the predicate becoming true (per Phase 3 decision - 3.10). + without the predicate becoming true. """ if watchdog is None: return if not (watchdog.done and not watchdog.success): return # Watchdog has tripped. Harvest diagnostics from the HDF - # plugin to annotate the exception (per Phase 3 decision - # 3.11 — keep raise behavior; RE will STOP movables). + # plugin to annotate the exception, then raise so the + # RunEngine STOPs movables. write_status = _safe_get(det.hdf1, "write_status", as_string=True) write_message = _safe_get(det.hdf1, "write_message", as_string=True) full_file_name = _safe_get(det.hdf1, "full_file_name", as_string=True) @@ -2271,16 +2240,13 @@ def flyscan( # rely on bluesky's RunEngine auto-derivation # (getattr(plan, "__name__", "")) because the @bluesky_plan # decorator wraps the generator in a Plan() class instance that - # has no __name__ attribute — that path yields plan_name="" - # (empirically confirmed 2026-06-10; see flyscan_3idc_notes.md). + # has no __name__ attribute — that path yields plan_name="". plan_name: str = "flyscan", # Seconds to add to each hdf1.array_counter monitor-stream # timestamp to obtain the corresponding frame's # start-of-acquire moment. None (the default) means "use - # -t_acquire" -- the Phase 0 verdict for the gp:m1 + - # adsimdet IOC: hdf_t timestamps each event at ~end_acquire, + # -t_acquire": hdf_t timestamps each event at ~end_acquire, # so start_acquire = hdf_t - t_acquire. See - # flyscan_3idc_notes.md for the empirical derivation and # flyscan_3idc_analysis.hdf_timestamp_semantic_diagnostic # for how to determine the right value on a different IOC. # Per-call override for IOCs/detectors with different HDF @@ -2288,23 +2254,22 @@ def flyscan( hdf_t_phase_offset: float = None, # TODO: trigger mode? # ------------------------------- internal parameters - # Wake-up tick for monitor_loop's consumer (Phase 3). CA + # Wake-up tick for monitor_loop's consumer. CA # monitor callbacks update status flags asynchronously; the # plan wakes up every _consumer_tick seconds to check them. # Default defined as a module-level constant so all related # timing knobs live in one place; can be overridden per-run. _consumer_tick: float = _CONSUMER_TICK_DEFAULT, # Override the HDF plugin's blocking_callbacks setting. - # Default (False) leaves the safe behaviour in place: HDF runs - # with blocking_callbacks="Yes" so the cam back-throttles to - # HDF's write rate and no frames are dropped. Setting True - # forces blocking_callbacks="No" on the HDF plugin, restoring - # the historical (data-losing) behaviour. The only legitimate - # use is to *demonstrate* the FlyscanDataLossWarning code path - # on hardware where blocking mode prevents drops — set True, - # crank up exposures_per_egu past what the HDF can sustain, - # and watch the post-scan warning fire. Not for production - # data collection. + # Default (False) keeps the safe mode: HDF runs with + # blocking_callbacks="Yes" so the cam back-throttles to HDF's + # write rate and no frames are dropped. Setting True forces + # blocking_callbacks="No", which lets the HDF plugin drop frames. + # The only legitimate use is to *demonstrate* the + # FlyscanDataLossWarning code path on hardware where blocking mode + # prevents drops — set True, crank up exposures_per_egu past what + # the HDF can sustain, and watch the post-scan warning fire. Not + # for production data collection. _force_hdf_nonblocking: bool = False, # ------------------------------- user-supplied metadata: always last md: dict = None, @@ -2598,8 +2563,7 @@ def flyscan( compute the three per-frame positions (start_acquire, end_acquire, end_period) recorded in the analysis output and (eventually) the NeXus master file. ``None`` (default) - means "use ``-t_acquire``" — the Phase 0 empirical verdict - for the gp:m1 + adsimdet IOC (``hdf_t`` arrives at + means "use ``-t_acquire``" (``hdf_t`` arrives at ~``end_acquire``, so ``start_acquire = hdf_t - t_acquire``). See ``flyscan_3idc_analysis.hdf_timestamp_semantic_diagnostic`` @@ -2651,8 +2615,7 @@ def flyscan( # t_acquire defaults to t_period (continuous exposure). if t_acquire is None: t_acquire = t_period - # hdf_t_phase_offset defaults to -t_acquire, the Phase 0 - # verdict for the gp:m1 + adsimdet IOC: hdf_t arrives at + # hdf_t_phase_offset defaults to -t_acquire: hdf_t arrives at # ~end_acquire, so start_acquire = hdf_t - t_acquire. if hdf_t_phase_offset is None: hdf_t_phase_offset = -t_acquire @@ -2740,11 +2703,10 @@ def flyscan( scan_velocity = geometry.scan_velocity # IOC HDF plugin pre-allocation overflows somewhere between 1e6 - # and 1e9 for Float64 1024x1024 frames (verified empirically; - # likely a C int byte-count overflow in NDFileHDF5). num_capture - # = num_frames * 1.5 + 20 is comfortable for any sensible scan - # size while absorbing takeoff & landing leading frames, post-stop - # tail frames, and timing jitter. + # and 1e9 for Float64 1024x1024 frames (likely a C int byte-count + # overflow in NDFileHDF5). num_capture = num_frames * 1.5 + 20 is + # comfortable for any sensible scan size while absorbing takeoff & + # landing leading frames, post-stop tail frames, and timing jitter. # # NOTE: the raw HDF5 file therefore holds MORE frames than # ``num_frames`` (pre-roll + post-stop tail). This is explained in @@ -2839,8 +2801,7 @@ def flyscan( # makes bps.read(det) include them and live displays plot them. # We restore in _cleanup so other plans against this detector see # whatever kind it was configured with by default (typically - # Kind.config or Kind.omitted for these counters). See Phase 3 - # decision 3.5a/3.5b in the strategy doc. + # Kind.config or Kind.omitted for these counters). saved_kinds = snapshot_kinds( det.cam.array_counter, det.hdf1.array_counter, @@ -3102,20 +3063,15 @@ def _main(): # even in continuous image_mode -- if a user had pre-set # cam.num_images to a small value (e.g. 50) via MEDM, the cam # would stop after that many frames regardless of motor - # position. Observed on 2026-06-15: cam pre-set to 50 capped - # acquisition at 50 frames. Setting num_images here via - # stage_sigs guarantees boundary detection is the only stop - # condition; the user's pre-scan value is snapshotted by the - # earlier snapshot_stage_sigs(det, det.cam, ...) call and - # restored on unstage by restore_stage_sigs. + # position (a cam pre-set to 50 caps acquisition at 50 frames). + # Setting num_images here via stage_sigs guarantees boundary + # detection is the only stop condition; the user's pre-scan + # value is snapshotted by the earlier snapshot_stage_sigs(det, + # det.cam, ...) call and restored on unstage by + # restore_stage_sigs. det.cam.stage_sigs["num_images"] = effective_num_images(t_period) det.hdf1.stage_sigs["num_capture"] = hdf_num_capture - # AD callback-chain throttling (revised 2026-06-08 after a - # flyscan reported only 59 of an expected 101 frames written - # to HDF, with cam.array_counter showing the cam itself ran - # at the requested 10 Hz — i.e. the cam produced 117 frames - # but HDF wrote only 59, and hdf1.dropped_arrays climbed by - # ~58 to confirm). Root cause: under + # AD callback-chain throttling. Under # ``blocking_callbacks="No"`` the cam doesn't wait for HDF to # consume the previous frame before producing the next; HDF's # input queue overflows when the file-write throughput is @@ -3207,11 +3163,10 @@ def _main(): ) @bpp.run_decorator(md=_md) def takeoff_and_monitor(): - # Takeoff ordering (revised 2026-06-08 after empirical - # observation that the cam delivered its first frame several - # seconds after Acquire=1, by which time the motor was already - # past p_start, costing the user a chunk of their requested - # frame budget): + # Takeoff ordering. The cam can deliver its first frame + # several seconds after Acquire=1; launching the motor before + # then would let it move past p_start, costing the user a + # chunk of their requested frame budget. So: # # 1. Start the cam acquiring (Acquire=1). # 2. Wait for the HDF plugin to receive its first frame @@ -3227,13 +3182,12 @@ def takeoff_and_monitor(): # # group="scan" registers the MoveStatus with the RunEngine so # the bps.wait(group="scan") below absorbs any post-scan - # motor settling after monitor_loop returns. See Phase 3 - # decisions 2.4/3.8 in the strategy doc. + # motor settling after monitor_loop returns. # Diagnostic: log what the IOC actually has for cam timings # right before we start. Helps catch staging defects (e.g. # acquire_period got overridden, or acquire_time > t_period - # silently capped by the IOC) without an empirical "why is - # my frame rate wrong" investigation. + # silently capped by the IOC) without a "why is my frame rate + # wrong" investigation. actual_acquire_time = _safe_get(det.cam, "acquire_time", use_monitor=False) actual_acquire_period = _safe_get(det.cam, "acquire_period", use_monitor=False) actual_image_mode = _safe_get( @@ -3267,7 +3221,7 @@ def takeoff_and_monitor(): # Without this check the run would otherwise time out 5 s later # in the first-frame wait below, with a misleading apstools # "Path '/' does not exist on IOC" error from the unwinding - # path. See sessions/2026-06-16/README.md for the probe data. + # path. yield from _check_cam_armed(det) # From this point on, _cleanup should treat the HDF plugin as # active (drain, flush, verify). If we never get here, @@ -3345,19 +3299,15 @@ def takeoff_and_monitor(): # rather than "cam never started". no_frames_timeout = max(scan_active_duration + 2 * t_period, 5.0) - # Status-based exit condition for monitor_loop (Phase 3 - # decisions 3.8 + 3.9, revised by 3.B.4 after empirical - # gates caught two false-early-exit defects, and revised - # again 2026-06-08 after a flyscan hang where the loop - # never exited because cam.acquire (== Acquire_RBV) stayed - # at 1 for >60s after we wrote Acquire=0 — the simulator - # IOC apparently finishes its current burst before the - # RBV drops. Replaced with cam.acquire_busy, which the - # IOC drops to 0 promptly when wait_for_plugins=Yes - # (which we set via stage_sigs); same signal - # wait_for_acquire_drained uses successfully in cleanup. - # Falls back to cam.acquire only on devices that don't - # expose acquire_busy. + # Status-based exit condition for monitor_loop. Uses + # cam.acquire_busy rather than cam.acquire: cam.acquire + # (== Acquire_RBV) can stay at 1 for many seconds after + # Acquire=0 is written (the IOC finishes its current burst + # before the RBV drops), which would hang the loop. + # cam.acquire_busy drops to 0 promptly when + # wait_for_plugins=Yes (set via stage_sigs); the same signal + # wait_for_acquire_drained uses in cleanup. Falls back to + # cam.acquire only on devices that don't expose acquire_busy. # This is an AndStatus of two sub-statuses: # 1. cam_stopped_status: cam.acquire_busy == 0 (preferred) # or cam.acquire == 0 (fallback). The busy signal @@ -3422,15 +3372,14 @@ def takeoff_and_monitor(): ) hdf_drain_status = AndStatus(cam_stopped_status, drain_status) - # No-frames watchdog (Phase 3 decision 3.10): a status that - # times out if num_captured doesn't reach > 0 within - # no_frames_timeout seconds. ophyd's StatusBase timeout - # mechanism sets the status to done-with-exception - # (StatusTimeoutError) on its own thread; the consumer in - # monitor_loop checks `watchdog_status.done and not - # watchdog_status.success` per tick to detect the trip and - # raise RuntimeError (per 3.11 — RE then sends STOP to all - # in-motion movables, including flymotor). + # No-frames watchdog: a status that times out if num_captured + # doesn't reach > 0 within no_frames_timeout seconds. ophyd's + # StatusBase timeout mechanism sets the status to + # done-with-exception (StatusTimeoutError) on its own thread; + # the consumer in monitor_loop checks `watchdog_status.done and + # not watchdog_status.success` per tick to detect the trip and + # raise RuntimeError (the RE then sends STOP to all in-motion + # movables, including flymotor). watchdog_status = SubscriptionStatus( det.hdf1.num_captured, lambda *, value, **_: int(value) > 0, @@ -3515,7 +3464,7 @@ def _cleanup(): # 4. Drain HDF queue (flush in-flight frames to plugin buffer) # 5. write_file=1 (force HDF5 file to disk; required because # auto_save=Yes does not flush when capture is stopped - # early, as we do here — verified empirically) + # early, as we do here) # 6. Verify full_file_name (the "tell" that the file landed) # 7. Restore overridden signals # 8. Restore stage_sigs snapshots @@ -3566,17 +3515,13 @@ def _cleanup(): # Verify the file landed on disk. We rely on auto_save=Yes # (set in the override list) to flush the HDF5 file when - # capture stops. Verified empirically: from a bluesky plan - # with auto_save=Yes, Capture=0 causes the IOC to write the - # file without our needing to set WriteFile=1. - # - # (Manual GUI use with auto_save=No is different: there, - # WriteFile=1 must be set explicitly after Capture=0. An - # earlier version of this code issued WriteFile=1 here as - # well, but it ran *after* the auto_save had already - # written the file, so the IOC rejected it with status=3. - # The file was fine; the error was spurious; the code was - # redundant.) + # capture stops: from a bluesky plan with auto_save=Yes, + # Capture=0 causes the IOC to write the file without our + # needing to set WriteFile=1. (Do NOT add WriteFile=1 here: + # with auto_save=Yes it runs after the file is already + # written and the IOC rejects it with status=3 — a spurious + # error on an otherwise-fine file. WriteFile=1 is only + # needed for manual GUI use with auto_save=No.) # # FullFileName_RBV is populated by the IOC after a # successful write. An empty value here is the "tell" diff --git a/src/id3c/utils/flyscan_3idc_analysis.py b/src/id3c/utils/flyscan_3idc_analysis.py index f5189ea..d02d31b 100644 --- a/src/id3c/utils/flyscan_3idc_analysis.py +++ b/src/id3c/utils/flyscan_3idc_analysis.py @@ -40,18 +40,14 @@ Design notes ------------ -- IOC timestamps are the system of record for pairing (per the - ``flyscan_3idc`` strategy doc, Phase 0.2 / Phase 0e). The +- IOC timestamps are the system of record for pairing. The primary-stream snapshots from the plan are a progress indicator; this module's output is the high-fidelity pairing. -- Empirically (verified during the 2026-06-08 commissioning - session against ``adsimdet`` + ``gp:m1``): - - the m1 monitor stream's record-order is interleaved across - multiple CA dispatcher segments; sorting by timestamp yields - a strictly increasing position trace at constant velocity in - the in-scan window. - - the HDF array_counter monitor stream's record-order is also - interleaved, but sorting by timestamp yields strictly +- Monitor-stream record order is interleaved across CA dispatcher + segments, so sort by timestamp: + - the motor monitor stream then yields a strictly increasing + position trace at constant velocity in the in-scan window. + - the HDF array_counter monitor stream then yields strictly monotonic counter values (0, 1, 2, ..., contiguous). - The function uses linear interpolation of motor position vs motor IOC timestamp. Linear is exact for a motor at constant @@ -126,8 +122,7 @@ def _interpolate_positions( OVERLAPS the time window during which the motor was inside ``[p_start, p_end]``. That window is bracketed by the first and last motor-stream samples whose position lies in - ``[p_start, p_end]``. This widening (over the older - "position_start_acquire inside [p_start, p_end]" rule) admits + ``[p_start, p_end]``. The time-overlap rule admits leading-edge and trailing-edge frames whose exposure crossed a boundary mid-way, as well as frames that fell on the wrong side of the boundary only due to motor-stream @@ -144,9 +139,8 @@ def _interpolate_positions( (``hdf_t`` arrives at or after the frame's cam-end-of- exposure event; ``start_acquire`` is one t_acquire earlier). See ``hdf_timestamp_semantic_diagnostic`` to determine the - right value empirically; ``flyscan_3idc.build_flyscan_md`` - defaults this to ``-t_acquire`` (the value Phase 0 derived - for the gp:m1 + adsimdet IOC). + right value for an IOC; ``flyscan_3idc.build_flyscan_md`` + defaults this to ``-t_acquire``. Returns ------- @@ -274,9 +268,9 @@ def _interpolate_positions( # A frame is "in scan" if the time interval over which it was # exposing/holding ([start_acquire.t, end_period.t]) OVERLAPS the # time window during which the motor was inside [p_start, p_end]. - # This is a deliberate widening over the older - # "position_start_acquire inside [p_start, p_end]" rule, which - # over-rejected three classes of frame: + # The time-overlap rule (rather than testing a single position + # against [p_start, p_end]) admits three classes of frame that + # carry valid in-range data: # # 1. Leading-edge: exposure started just before p_start but # crossed p_start before end_acquire. The frame DOES carry @@ -297,14 +291,12 @@ def _interpolate_positions( # # Frames that never overlapped the window at all (taxi-in / coast- # out frames whose entire [start_acquire, end_period] falls before - # the first in-range motor sample or after the last) are still - # dropped, which is the correct behaviour for the original taxi / - # coast rejection use case. + # the first in-range motor sample or after the last) are dropped: + # they carry no in-range data. in_range_pos = (m_p >= p_start) & (m_p <= p_end) if not in_range_pos.any(): # The motor never entered the scan range in this run; reject - # every frame. This branch was implicitly handled before by - # the strict-position check; preserved here for symmetry. + # every frame. in_scan = np.zeros_like(ts_start, dtype=bool) motor_t_in_range_start = None motor_t_in_range_end = None From 7f2cc170e689c36875ed779bb4a5c53f52572d2d Mon Sep 17 00:00:00 2001 From: Pete Jemian Date: Wed, 24 Jun 2026 12:51:48 -0500 Subject: [PATCH 8/8] DOC: declutter flyscan() signature and constant docstrings - Reduce the flyscan() signature to a clean kwarg list; move the per-kwarg detail to the docstring Parameters section (and document _force_hdf_nonblocking there, which had been inline only). - Convert module-level constant comments to following docstrings (_CONSUMER_TICK_DEFAULT, _CLEANUP_DRAIN_TICK, _FRAME_QUEUE_SIZE, _ACCL_FALLBACK_SECONDS, _AD_FILES_WARNED, _CAM_ERROR_STATES). - Split dense multi-topic paragraphs into one topic per paragraph (flyscan() summary, read_motor_field, _FRAME_QUEUE_SIZE). --- src/id3c/plans/flyscan_3idc.py | 133 +++++++++++++++------------------ 1 file changed, 60 insertions(+), 73 deletions(-) diff --git a/src/id3c/plans/flyscan_3idc.py b/src/id3c/plans/flyscan_3idc.py index 2181f1c..5686135 100644 --- a/src/id3c/plans/flyscan_3idc.py +++ b/src/id3c/plans/flyscan_3idc.py @@ -156,24 +156,32 @@ def effective_num_images(t_period: float) -> int: # Adjust here rather than at call sites — keeps related knobs together # and discoverable. -# Default wake-up tick for monitor_loop's consumer (also the default -# for the flyscan(_consumer_tick=...) plan kwarg). 20 ms = 50 Hz. _CONSUMER_TICK_DEFAULT = 0.02 +"""Default wake-up tick for monitor_loop's consumer. + +Also the default for the ``flyscan(_consumer_tick=...)`` plan kwarg. +20 ms = 50 Hz. +""" -# Wake-up tick for wait_for_acquire_drained in the cleanup path. -# Cleanup latency does not matter for live progress, so coarser is -# fine; 50 ms = 20 Hz. _CLEANUP_DRAIN_TICK = 0.05 +"""Wake-up tick for wait_for_acquire_drained in the cleanup path. + +Cleanup latency does not matter for live progress, so coarser is +fine; 50 ms = 20 Hz. +""" -# Bounded size for monitor_loop's per-frame producer/consumer queue. -# The producer (a CA monitor callback on hdf1.num_captured) pushes -# (timestamp, value) tuples; the consumer drains in monitor_loop. -# At typical scan rates (10 Hz frames, 20 ms consumer tick), the -# queue stays nearly empty. This bound exists to detect a -# producer/consumer mismatch — overflow is logged once as a WARNING -# and the entry is dropped. Increase -# here if higher-rate scans are ever attempted. _FRAME_QUEUE_SIZE = 64 +"""Bounded size for monitor_loop's per-frame producer/consumer queue. + +The producer (a CA monitor callback on hdf1.num_captured) pushes +(timestamp, value) tuples; the consumer drains in monitor_loop. At +typical scan rates (10 Hz frames, 20 ms consumer tick), the queue +stays nearly empty. + +This bound exists to detect a producer/consumer mismatch — overflow +is logged once as a WARNING and the entry is dropped. Increase here +for higher-rate scans. +""" # Public API of this module. Other symbols (validators, @@ -243,10 +251,12 @@ def read_motor_field(motor, suffix, timeout=1.0): Uses ``epics.caget`` directly rather than constructing a throwaway ``EpicsSignal``. ``caget`` runs on the calling thread's CA context - and does no asynchronous metadata fetching; a throwaway - ``EpicsSignal`` can segfault when pyepics fires a deferred metadata - callback after the signal's CA channel is torn down, and would also - need suppression to avoid polluting the global oregistry. + and does no asynchronous metadata fetching. + + A throwaway ``EpicsSignal`` is avoided because it can segfault when + pyepics fires a deferred metadata callback after the signal's CA + channel is torn down, and it would also need suppression to avoid + polluting the global oregistry. """ pv = motor.prefix + suffix try: @@ -723,10 +733,12 @@ def configure_adsimdet( return result -# Fallback acceleration (seconds) used when ``.ACCL`` cannot be read. -# Deliberately generous; over-allocating the taxi region only costs a -# bit of extra travel before the first useful frame. _ACCL_FALLBACK_SECONDS = 0.25 +"""Fallback acceleration (seconds) used when ``.ACCL`` cannot be read. + +Deliberately generous; over-allocating the taxi region only costs a +bit of extra travel before the first useful frame. +""" class FlyscanDataLossWarning(UserWarning): @@ -1260,8 +1272,8 @@ def restore_stage_sigs(snapshot): dev.stage_sigs.update(saved) -# Once-per-(master_dir, target) dedup for _check_ad_files_symlink. _AD_FILES_WARNED = set() +"""Once-per-(master_dir, target) dedup set for _check_ad_files_symlink.""" def _read_path_template(det): @@ -1511,10 +1523,11 @@ def _expected_frame_count( return None -# Cam state values (DetectorState_RBV enum) that indicate the cam -# failed to arm or is otherwise unusable. On the Eiger a failed arm -# transitions to 'Error' within ~2 ms. _CAM_ERROR_STATES = {"Error", "Aborted", "Aborting", "Disconnected"} +"""Cam DetectorState_RBV values meaning the cam failed to arm / is unusable. + +On the Eiger a failed arm transitions to 'Error' within ~2 ms. +""" def _check_cam_armed(det, poll_s=0.05, max_wait_s=0.5): @@ -2215,8 +2228,6 @@ def _check_watchdog(): @bluesky_plan def flyscan( - # Extra readables, reported each frame. Self-updating only: - # the plan does NOT call .trigger() on these. detectors: list = None, det_name: str = "adsimdet", flymotor_name: str = "m1", @@ -2224,68 +2235,35 @@ def flyscan( p_end: float = 5, exposures_per_egu: float = 2.0, t_period: float = 0.1, - t_acquire: float = None, # defaults to t_period when None - taxi_allowance: float = 0.5, # motor EGU + t_acquire: float = None, + taxi_allowance: float = 0.5, compression: str = "zlib", ad_file_name: str = "flyscan", ad_file_path: str = "/tmp/flyscan", - # Effective scan-velocity floor is max(.VBAS, velocity_minimum); - # leaving this at None defers to .VBAS alone. See - # validate_flyscan_inputs for the bracket policy. velocity_minimum: float = None, - # plan_name appears in the run's start document and in the NeXus - # master file. Wrapper plans should pass their own name here - # (e.g. plan_name="my_3idc_scan") so the run's provenance - # reflects the wrapper, not the inner flyscan() call. We can't - # rely on bluesky's RunEngine auto-derivation - # (getattr(plan, "__name__", "")) because the @bluesky_plan - # decorator wraps the generator in a Plan() class instance that - # has no __name__ attribute — that path yields plan_name="". plan_name: str = "flyscan", - # Seconds to add to each hdf1.array_counter monitor-stream - # timestamp to obtain the corresponding frame's - # start-of-acquire moment. None (the default) means "use - # -t_acquire": hdf_t timestamps each event at ~end_acquire, - # so start_acquire = hdf_t - t_acquire. See - # flyscan_3idc_analysis.hdf_timestamp_semantic_diagnostic - # for how to determine the right value on a different IOC. - # Per-call override for IOCs/detectors with different HDF - # plugin timestamp semantics. hdf_t_phase_offset: float = None, - # TODO: trigger mode? - # ------------------------------- internal parameters - # Wake-up tick for monitor_loop's consumer. CA - # monitor callbacks update status flags asynchronously; the - # plan wakes up every _consumer_tick seconds to check them. - # Default defined as a module-level constant so all related - # timing knobs live in one place; can be overridden per-run. + # Internal parameters (underscore-prefixed); see Parameters. _consumer_tick: float = _CONSUMER_TICK_DEFAULT, - # Override the HDF plugin's blocking_callbacks setting. - # Default (False) keeps the safe mode: HDF runs with - # blocking_callbacks="Yes" so the cam back-throttles to HDF's - # write rate and no frames are dropped. Setting True forces - # blocking_callbacks="No", which lets the HDF plugin drop frames. - # The only legitimate use is to *demonstrate* the - # FlyscanDataLossWarning code path on hardware where blocking mode - # prevents drops — set True, crank up exposures_per_egu past what - # the HDF can sustain, and watch the post-scan warning fire. Not - # for production data collection. _force_hdf_nonblocking: bool = False, - # ------------------------------- user-supplied metadata: always last + # User-supplied metadata: always last. md: dict = None, ): """Fly scan: move motor through range while acquiring detector frames. The motor traverses ``p_initial → ≤ p_final``, maintaining constant velocity between ``p_start → p_end`` to deliver ``num_frames`` frames - within ``[p_start, p_end]``. ``p_initial`` and ``p_final`` are computed - from ``p_start``, ``p_end``, the motor's ``.ACCL``, and + within ``[p_start, p_end]``. ``p_initial`` and ``p_final`` are + computed from ``p_start``, ``p_end``, the motor's ``.ACCL``, and ``taxi_allowance``; ``num_frames`` is computed from - ``(p_end - p_start) * exposures_per_egu``. Detector frames are - acquired continuously during the traverse; downstream processing - trims the data to ``[p_start, p_end]`` by motor position. An HDF5 - file containing every captured frame is written next to the run - (the path is in the run metadata under ``ad_file_path`` / + ``(p_end - p_start) * exposures_per_egu``. + + Detector frames are acquired continuously during the traverse; + downstream processing trims the data to ``[p_start, p_end]`` by + motor position. + + An HDF5 file containing every captured frame is written next to the + run (the path is in the run metadata under ``ad_file_path`` / ``ad_file_name``). Position geometry @@ -2573,6 +2551,15 @@ def flyscan( Increase if your run-engine subscriptions can't keep up; decrease only for very high frame rates. Rarely needs to be changed. + _force_hdf_nonblocking : bool, default ``False`` + Internal/diagnostic. ``False`` keeps the safe mode + (``blocking_callbacks="Yes"``), where the cam back-throttles + to the HDF write rate so no frames are dropped. ``True`` + forces ``blocking_callbacks="No"``, letting the HDF plugin + drop frames. Its only legitimate use is to demonstrate the + ``FlyscanDataLossWarning`` code path: set ``True``, push + ``exposures_per_egu`` past what the HDF can sustain, and watch + the post-scan warning fire. Not for production data collection. md : dict, optional Additional metadata to record under the run's ``start`` document. Merged on top of the plan's computed metadata.