diff --git a/AGENTS.md b/AGENTS.md index 4d3bbb5f..076eb750 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -95,7 +95,9 @@ src/morphobricks_preprocessing/ axes.py AxisMapping (role per dimension), prefill, check_mapping, review_axes (one mapping applied to every file, missing axes as singletons only when confirmed) axis_review.py review_axes and check_mapping (moved from axes.py in PP-4) - fingerprint.py Fingerprint (size, mtime) and RowKey: when a file's rows are reused (D5); + inventory.py source_signature/source_fingerprint: a file's size and mtime, an OME-Zarr + folder's total size, newest mtime and inventory digest (issue #86; PP-12) + fingerprint.py Fingerprint (size, mtime, inventory) and RowKey: when a file's rows are reused (D5); confirm_axes/unchanged_confirmation (a confirmation of the files as they are) recorded_sizes.py RecordedSize: pixel sizes by role with unit and source (manifest v2): header, stated or pixels (no value) @@ -110,6 +112,7 @@ src/morphobricks_preprocessing/ unrecorded.py unrecorded_files: images and masks the run neither recorded nor excluded, by preprocessing's discovery and pairing; RunInputs.open raises UnrecordedFilesError for them (PP-8b, contract §9.1) + run_inputs_text.py the texts RunInputs refuses a run with (moved from run_inputs.py in PP-12) mask_record.py the manifest's masks entry with fingerprints and roles (manifest v2) axis_check.py the axis check (contract §2): header_axis_issue, axis_disagreements, likely_axis_cause, check_header_axes, ConfirmedAxes (a confirmation covers @@ -133,7 +136,11 @@ src/morphobricks_preprocessing/ describe_masks, recorded_masks; PP-10d), result.py (RunResult), entries.py (manifest entries, new/changed/unchanged by RowKey), reuse.py (which earlier rows are reused), rows.py (a file's rows and their - pixel checks), publish.py (staged outputs, published together) + pixel checks), publish.py (write_report), generation.py (the outputs + as one generation: .publish, the manifest's replacement as the commit, + finish_publication, published_path/read_stable for readers; PP-12), + staging.py (each file's rows staged in .staging), resume.py (resume or + read each file between fingerprints, source_changed_during_run) report.py plain-language summaries shared by CLI and notebook, group statistics tables mask_text.py the masks' texts (mask_summary, mask_report_summary) and the notebook's tables of the mask step (mask_type_rows, mask_relationship_rows, @@ -165,8 +172,8 @@ src/morphobricks_preprocessing/ under the confirmed roles, blocks written in place (BigTIFF when large) conversion_record.py the copy tree's .morphobricks-conversion.json (version 2: fingerprints, copy SHA-256, roles, sizes, channel names, bits), review_conversion (the - run's conversion_source_changed and copy_changed_after_conversion - warnings), changed_copies + run's conversion_source_changed warning and copy_changed_after_conversion + error, PP-12), changed_copies content_hash.py content_sha256: a copy's SHA-256 (file, or OME-Zarr folder without labels) conversion_check.py conversion_problem: RunInputs.open's check of a copy tree (record SHA-256, version, each recorded copy's SHA-256, read once at open) @@ -427,7 +434,8 @@ functions the tests use. stays 1); flags are JSON booleans only and the versions are strings (`"1"`, `"4"`); - channel names and significant bits (OME only, information only); - the CSV's SHA-256 and package versions (including `morphobricks-normalization`). -- **A file is unchanged** when its size, mtime, labels and axis layout match and the table +- **A file is unchanged** when its size, mtime (an OME-Zarr image: also its inventory digest, + `inventory_sha256`, issue #86), labels and axis layout match and the table still holds all its rows. Unchanged rows are reused; new and changed files are read; deleted, renamed or excluded files lose their rows. - **Full rebuild** happens in `mode="full"` (PP-7; `force_rebuild` was removed), when there @@ -435,9 +443,14 @@ functions the tests use. the dtype, bin count, histogram settings (`RunSettings.histogram`) or axis mapping changed, or when the CSV's hash no longer matches. Changing only the memory limit or the exclusions recomputes nothing. -- **Writes:** the CSV is written first, then the manifest, each via a temporary file and - replace. Nothing is written if any error is found. An interrupted run is caught next - time by the hash check. +- **Writes (PP-12, contract §9.4):** each image's rows are staged in `.staging` as soon + as it is read (an interrupted run resumes from the staged rows whose key still matches; + anything else is discarded and reported). The outputs are one generation, written into + `.publish` and committed by the manifest's `os.replace` (the one atomic step), then moved + into place; the next holder of the lock finishes or discards an interrupted publication. + Every image is fingerprinted when its header is read, before and after its pixels are + read, and before the commit (`source_changed_during_run`, issue #90). Nothing is + published if any error is found. - **Exclusions:** the user may exclude images explicitly (`exclude=[...]`, e.g. damaged files). Excluded images are not read, are reported (`excluded_images` warning), are recorded in the manifest, and lose their rows. diff --git a/docs/contract.md b/docs/contract.md index 32c01fd2..60186a3c 100644 --- a/docs/contract.md +++ b/docs/contract.md @@ -122,8 +122,8 @@ method is an offset and a scale, `(pixels − offset) / scale`. | Severity | Situations | Effect | |---|---|---| -| Error | Experiment design not confirmed (`design_not_confirmed`) or a repeat group holding two folders of one condition (`repeat_group_conflict`), when a design is used (§12); image type RGB for images that are not three channels of an integer type (`rgb_not_possible`, §13); mixed dtypes; images with different numbers of channels (`different_channel_counts`, §2) or different channel names at the same channel number (`channel_names_differ`, §2); unreadable TIFF (header or pixel data) or OME-Zarr folder in a leaf folder that is not excluded; OME-Zarr images without the `zarr` extra; OME-Zarr images whose declared axes do not fit the experiment's roles or that the confirmed roles contradict (`ome_zarr_axes_mismatch`, §2); OME-Zarr images that do not name their axes, and every OME-NGFF 0.1 or 0.2 store (`unreadable_ome_zarr`, §2); every image excluded; more than one image series in a file; unsupported sample format or axis; missing/duplicate X or Y, duplicate roles, unassigned non-singleton axis, inconsistent mapping; an image whose confirmed roles swap Y and X, contradict its header or put X before Y (`axes_yx_swapped`, `axes_contradict_header`, `axes_x_before_y`, §2); plane that cannot be read within the memory limit, not even in blocks (for example a plane stored compressed in one piece); folder named `single` in another case, or beside other folders (§1); folders of one level differing only in case, anywhere in the experiment (`case_conflict`); folder or image names with outer whitespace or reading as a missing value (`invalid_name`, §1); images in one leaf folder with the same position identifier (§1); links that leave the experiment or masks folder, loop, or reach one file twice (`link_outside_folder`, `link_loop`, `linked_twice`, §1); folder below the technical-repeat level; images in a folder that also holds subfolders (`tiff_outside_leaf`, §1); output folder inside the experiment or inside the masks folder (`output_inside_masks`, §15); images named as masks of other images (`image_named_as_mask`, §1); no usable planes; no axis roles given or recorded (`axes_not_confirmed`, §9.2); another run holding the output folder's lock (`output_locked`, §9.2); a copy tree whose conversion record cannot be read (`conversion_record_unreadable`, §17); a recorded `experiment_design.csv` that cannot be read (`design_unreadable`); a `calibration.csv` that cannot be read (`calibration_unreadable`) or that states sizes for a target that is neither a folder nor an image of the experiment (`calibration_target_not_found`, §9.3); no experiment folder given or recorded (`experiment_not_recorded`, §9.2); when masks are given, a masks folder missing or inside the experiment, mask names differing only in case, two masks of one type for an image, a mask whose name could belong to two images or types, or a mask that is unreadable, has axes other than Y, X and named or stated Z/T (`mask_not_2d`), has stated axes that do not fit it or contradict the axes its metadata names (`invalid_mask_axes`, `mask_axes_contradict_header`), is stored with X before Y (its header's or its stated axes; refused by `check` and `run` too, never recorded with empty axes) or with its image's Y and X sizes swapped (`mask_layout`; never transposed), is of another size than its image (in `run`, its image's Y and X under the confirmed roles; not reported for an image with an axis error), is not of integers or booleans, has other depth slices or time points than the image's other masks or, in `run`, than the image (§15) | Processing is blocked | -| Warning | Images whose metadata does not name their axes, read with the confirmed roles (`axes_not_named`, §2); in a tree of named-axis copies, sources changed or gone since the conversion (`conversion_source_changed`) and copies changed, gone or added since (`copy_changed_after_conversion`, §17); folders named like a masks folder holding images named as masks (`mask_folder_in_experiment`, §1); intensity checks of the written table (`image_intensity_outlier`, `repeat_intensity_shift`, `saturated_image`, `no_intensity_spread`, §14), reported apart from the run's issues; files that are not images; empty folders; images excluded by the user, or excluded paths that match no image; non-finite pixels; different image widths or heights; different numbers of depth slices, time points or other planes; physical pixel sizes that differ or are known for only some images; a stated pixel size that differs from the header (`calibration_differs_from_header`, §9.3; the stated size is used); unbalanced numbers of images; biological repeats missing from some conditions (§11); planes without finite pixels while others remain; channels whose central intensity range covers too few histogram bins (`narrow_histogram_range`, §4); planes over the memory limit that are read in blocks; images without a mask of a type other images have, masks matching no image, masks of excluded images, files outside the mask-type folders (§15) | Processing continues; the item is reported | +| Error | Experiment design not confirmed (`design_not_confirmed`) or a repeat group holding two folders of one condition (`repeat_group_conflict`), when a design is used (§12); image type RGB for images that are not three channels of an integer type (`rgb_not_possible`, §13); mixed dtypes; images with different numbers of channels (`different_channel_counts`, §2) or different channel names at the same channel number (`channel_names_differ`, §2); unreadable TIFF (header or pixel data) or OME-Zarr folder in a leaf folder that is not excluded; OME-Zarr images without the `zarr` extra; OME-Zarr images whose declared axes do not fit the experiment's roles or that the confirmed roles contradict (`ome_zarr_axes_mismatch`, §2); OME-Zarr images that do not name their axes, and every OME-NGFF 0.1 or 0.2 store (`unreadable_ome_zarr`, §2); every image excluded; more than one image series in a file; unsupported sample format or axis; missing/duplicate X or Y, duplicate roles, unassigned non-singleton axis, inconsistent mapping; an image whose confirmed roles swap Y and X, contradict its header or put X before Y (`axes_yx_swapped`, `axes_contradict_header`, `axes_x_before_y`, §2); plane that cannot be read within the memory limit, not even in blocks (for example a plane stored compressed in one piece); folder named `single` in another case, or beside other folders (§1); folders of one level differing only in case, anywhere in the experiment (`case_conflict`); folder or image names with outer whitespace or reading as a missing value (`invalid_name`, §1); images in one leaf folder with the same position identifier (§1); links that leave the experiment or masks folder, loop, or reach one file twice (`link_outside_folder`, `link_loop`, `linked_twice`, §1); folder below the technical-repeat level; images in a folder that also holds subfolders (`tiff_outside_leaf`, §1); output folder inside the experiment or inside the masks folder (`output_inside_masks`, §15); images named as masks of other images (`image_named_as_mask`, §1); no usable planes; no axis roles given or recorded (`axes_not_confirmed`, §9.2); another run holding the output folder's lock (`output_locked`, §9.2); a copy tree whose conversion record cannot be read (`conversion_record_unreadable`, §17), or whose copies changed, went missing or were added since the conversion (`copy_changed_after_conversion`, §17; owner decision of 2 November 2026, PP-12: the bricks would refuse the run); an image that changed or went missing while the run checked or read it (`source_changed_during_run`, §9.4; nothing is published); a recorded `experiment_design.csv` that cannot be read (`design_unreadable`); a `calibration.csv` that cannot be read (`calibration_unreadable`) or that states sizes for a target that is neither a folder nor an image of the experiment (`calibration_target_not_found`, §9.3); no experiment folder given or recorded (`experiment_not_recorded`, §9.2); when masks are given, a masks folder missing or inside the experiment, mask names differing only in case, two masks of one type for an image, a mask whose name could belong to two images or types, or a mask that is unreadable, has axes other than Y, X and named or stated Z/T (`mask_not_2d`), has stated axes that do not fit it or contradict the axes its metadata names (`invalid_mask_axes`, `mask_axes_contradict_header`), is stored with X before Y (its header's or its stated axes; refused by `check` and `run` too, never recorded with empty axes) or with its image's Y and X sizes swapped (`mask_layout`; never transposed), is of another size than its image (in `run`, its image's Y and X under the confirmed roles; not reported for an image with an axis error), is not of integers or booleans, has other depth slices or time points than the image's other masks or, in `run`, than the image (§15) | Processing is blocked | +| Warning | Images whose metadata does not name their axes, read with the confirmed roles (`axes_not_named`, §2); in a tree of named-axis copies, sources changed or gone since the conversion (`conversion_source_changed`, §17); folders named like a masks folder holding images named as masks (`mask_folder_in_experiment`, §1); intensity checks of the written table (`image_intensity_outlier`, `repeat_intensity_shift`, `saturated_image`, `no_intensity_spread`, §14), reported apart from the run's issues; files that are not images; empty folders; images excluded by the user, or excluded paths that match no image; non-finite pixels; different image widths or heights; different numbers of depth slices, time points or other planes; physical pixel sizes that differ or are known for only some images; a stated pixel size that differs from the header (`calibration_differs_from_header`, §9.3; the stated size is used); unbalanced numbers of images; biological repeats missing from some conditions (§11); planes without finite pixels while others remain; channels whose central intensity range covers too few histogram bins (`narrow_histogram_range`, §4); planes over the memory limit that are read in blocks; images without a mask of a type other images have, masks matching no image, masks of excluded images, files outside the mask-type folders (§15) | Processing continues; the item is reported | Every message states what happened in plain language, lists the affected paths, and suggests an action. Technical details go to the diagnostic log. The command line's log @@ -133,8 +133,8 @@ earlier run in the output folder), inside `convert`'s folder of copies, as the d given with `--design` (an input of the run), as a folder or in a folder that does not exist, and as or inside an output artefact (`plane_histograms.csv`, `run_manifest.json`, the report files, `experiment_design.csv`, `calibration.csv`, -`mask_relationships.json`, the lock, `.morphobricks-conversion.json`, the `.run-` staging -folders; `log_target.PROTECTED_OUTPUTS`). Paths are compared resolved and, for the folders that exist, by +`mask_relationships.json`, the lock, `.morphobricks-conversion.json`, the publication and +staging folders `.publish` and `.staging` (§9.4); `log_target.PROTECTED_OUTPUTS`). Paths are compared resolved and, for the folders that exist, by device and inode (another spelling of a folder on a file system that ignores case); output names ignore case; an existing file with a second name (a hard link) is refused. Any other name in the output folder is allowed. @@ -147,14 +147,10 @@ other messages. ## 9. Run manifest (`run_manifest.json`, manifest version 2) -UTF-8 JSON next to `plane_histograms.csv`, written after the table and only when the run -succeeded. - -Both files are prepared in a temporary folder before either earlier output is replaced. -Earlier files are backed up before publication. A caught replacement failure restores the -previous state, including the absence of files on a first run. If restoration fails, the -backup folder is retained and its location is reported for manual recovery. Failures to -remove temporary files after a successful publication are logged. +UTF-8 JSON next to `plane_histograms.csv`, written only when the run succeeded. Its +replacement is the commit of the run's outputs: they are one **generation**, published +crash-consistently (v0.4.0, PP-12, owner decision of 2 November 2026; §9.4), and the +manifest names the table and the design by their SHA-256. **File permissions (v0.4.0, plan §12 question 52).** Every file the brick writes (the table, the manifest, the design, the report files, `calibration.csv` and @@ -165,10 +161,9 @@ user set it. Before v0.4.0 every output was `0600` (owner only) whatever the uma the temporary file of the atomic write kept its private mode; such files keep `0600` until the user changes them (`chmod a+r`) or deletes them. On Windows file modes do not apply. -The two replacements are not jointly atomic: forced process termination between them can -leave a mismatched pair, detected by `csv_sha256` on the next run and rebuilt. Only one run -writes to a given output folder at a time, enforced by a lock (§9.2). This write protocol -does not change either file's schema or statistical interpretation. +Only one run writes to a given output folder at a time, enforced by a lock (§9.2). The +publication protocol (§9.4) does not change any file's schema or statistical +interpretation. | Key | Meaning | |---|---| @@ -179,7 +174,7 @@ does not change either file's schema or statistical interpretation. | `settings` | `bin_count`, `memory_limit` (bytes), `missing_as_singleton`, `exclude` (sorted relative paths), `histogram` (`HistogramSettings.to_dict()` of the normalization brick: `library_version`, `histogram_range`, `robust_quantiles`, `min_central_bins`, §4; the default `minmax` range when absent, in manifests written before follow-up P6). | | `axis_mapping` | `axes` (file axis labels) and `roles` (`X`, `Y`, `Z`, `C`, `T` or null per axis). | | `labels` | One entry per leaf folder: `folder` (relative path, `.` for the root) and its three labels. | -| `files` | One entry per analysed file, sorted by path: `relative_path`, `size` (bytes), `mtime_ns`, the three labels, `axes`, `singleton_roles`, `plane_count` (its rows in the table: XYC planes), `channel_names` (list, null for unnamed channels; empty if none in metadata), `significant_bits` (null if not in metadata), `read_in_blocks` (true when the image's planes are over the memory limit and are read in blocks; information only, false when absent), `position_id` (the image's position identifier, §1; null when absent, in manifests written before it), `channel_names_trimmed` (true when leading or trailing whitespace was removed from the image's channel names as they were read, §2; `channel_names` holds the trimmed names; false when absent), `roles` (the role of each of the file's `axes`, `X`, `Y`, `Z`, `C`, `T` or null, from the file's own mapping: an OME-Zarr image in another order than `axis_mapping` has its own roles, so later bricks read these rather than rebuild them from `axis_mapping`; null when absent, in manifests written before them; optional key, version 1). For an OME-Zarr image, `size` is the total size of its files and `mtime_ns` the time of its newest file. **Version 2 (v0.4.0):** `roles` is required, with `axes_from` (required): `header` when the metadata names every axis and the roles agree, `confirmed` for a plain stack whose roles the user confirmed (§2); and `pixel_sizes`, one entry per role `X`, `Y`, `Z`, `T` with `value`, `unit` (OME-NGFF name) and `source`: `header` (the image's metadata) or `stated` (`calibration.csv`, §9.3) with a value, or `pixels` (stated as deliberately uncalibrated) with `value` and `unit` null; `value` and `source` null for an axis nothing gives a size (never guessed). A `header` or `stated` size without a value, or a value with no source or with `pixels`, is refused. The reader refuses a version 2 entry without `roles` or `axes_from`, and the writer refuses to write one. | +| `files` | One entry per analysed file, sorted by path: `relative_path`, `size` (bytes), `mtime_ns`, the three labels, `axes`, `singleton_roles`, `plane_count` (its rows in the table: XYC planes), `channel_names` (list, null for unnamed channels; empty if none in metadata), `significant_bits` (null if not in metadata), `read_in_blocks` (true when the image's planes are over the memory limit and are read in blocks; information only, false when absent), `position_id` (the image's position identifier, §1; null when absent, in manifests written before it), `channel_names_trimmed` (true when leading or trailing whitespace was removed from the image's channel names as they were read, §2; `channel_names` holds the trimmed names; false when absent), `roles` (the role of each of the file's `axes`, `X`, `Y`, `Z`, `C`, `T` or null, from the file's own mapping: an OME-Zarr image in another order than `axis_mapping` has its own roles, so later bricks read these rather than rebuild them from `axis_mapping`; null when absent, in manifests written before them; optional key, version 1). For an OME-Zarr image, `size` is the total size of its files, `mtime_ns` the time of its newest file, and `inventory_sha256` (v0.4.0, PP-12, issue #86; only for OME-Zarr images) the SHA-256 of its inventory: one line per file (its path relative to the image with `/`, a NUL, its size, a NUL, its `mtime_ns`, a newline), sorted by path, its root `labels` group left out; links to folders inside the image are followed, as the reader follows them, and links that reach one folder twice are refused. Adding, removing, renaming, resizing or touching any chunk or metadata file changes it; an OME-Zarr image recorded without it (v0.4.0rc3 and earlier) counts as changed (recomputed; `RunInputs` refuses it). Like a TIFF file's size and time it is metadata only (design v0.4 §1): a file rewritten with the same size and its time set back is not seen, only hashing the content would. **Version 2 (v0.4.0):** `roles` is required, with `axes_from` (required): `header` when the metadata names every axis and the roles agree, `confirmed` for a plain stack whose roles the user confirmed (§2); and `pixel_sizes`, one entry per role `X`, `Y`, `Z`, `T` with `value`, `unit` (OME-NGFF name) and `source`: `header` (the image's metadata) or `stated` (`calibration.csv`, §9.3) with a value, or `pixels` (stated as deliberately uncalibrated) with `value` and `unit` null; `value` and `source` null for an axis nothing gives a size (never guessed). A `header` or `stated` size without a value, or a value with no source or with `pixels`, is refused. The reader refuses a version 2 entry without `roles` or `axes_from`, and the writer refuses to write one. | | `csv_sha256` | SHA-256 of the table written with this manifest. | | `experiment_design_sha256` | SHA-256 of the `experiment_design.csv` written with this run (§12), so later bricks can check that the design they read is the one used; null when the run had no design, or absent in manifests written before it. Optional key: the manifest stays version 1. | | `masks` | The masks paired with the images (§15), when the run was given a masks folder or its OME-Zarr images have label images (always used): `folder` (absolute path of the masks folder, null when only OME-Zarr label images were used), `types` (sorted mask-type names) and `files` (one entry per image and mask type, sorted by image then type: `image`, `type`, `path`, `source` `folder` or `ome-zarr`, `axes` and `shape`: the mask's axes and lengths without its length-1 axes other than Y and X, `YX` for a 2D mask (`[1, 512]` for a mask one pixel high), with `Z` and/or `T` for a mask with depth slices or time points, e.g. `TYX` and `[50, 512, 512]`; §15). Absent when the run had no masks. Optional key: the table does not depend on the masks. `axes` and `shape` are absent in manifests written before them, whose masks are all 2D; a later brick must read `axes` before using a mask and never apply a 2D mask to several planes unless `shared` is true. **Version 2:** `axes` are the mask's axes as stored (its header's letters: `IYX` for a plain stack), and each mask entry also has `size` and `mtime_ns` (its fingerprint, as `source_fingerprint` gives it: the mask file, or the label image's folder), `roles` (one per axis of `axes`, what each is read as: `TYX` for that plain stack stated `--mask-axes Cells=TYX`), `axes_from` (`header`, or `stated` when the axes were stated for its mask type) and `shared` (true only for a 2-D mask stated `YX` for its type: one plane shared by every plane of its image). A mask is read as stored, never reordered. | @@ -190,7 +185,7 @@ does not change either file's schema or statistical interpretation. | `conversion_record_sha256` | SHA-256 of the conversion record (`.morphobricks-conversion.json`) in the experiment root when the experiment is a tree of named-axis copies (design v0.4 §5, §17); null otherwise. `RunInputs` checks it (owner decision of 2 November 2026, plan question 59): a record changed or gone since the run is refused, as for `calibration_sha256`. A changed record changes no row (the copies are checked by their fingerprints like any image). | | `calibration_sha256` | SHA-256 of the output folder's `calibration.csv` as the run read it (§9.3); null when there was none (or absent in manifests written before PP-8). `RunInputs` checks it. | | Version 1 | A manifest written before v0.4.0 (`"1"`) is still read, for reporting, but no row of it is reused: its axes were never checked. The next run recomputes every row ("the earlier run was written before v0.4.0, without the axis check"). Bricks refuse it. | -| Row reuse | A file's rows are reused only when its row key is the earlier run's (`fingerprint.RowKey`, one rule for "the same file, read the same way", audit D5): its fingerprint (`fingerprint.Fingerprint`, size and modification time), labels, axes, roles, `singleton_roles`, `axes_from` and the path it is read from (so moving or converting the experiment recomputes it), and the settings that change rows (bins, histogram range, robust quantiles, range-check limit). | +| Row reuse | A file's rows are reused only when its row key is the earlier run's (`fingerprint.RowKey`, one rule for "the same file, read the same way", audit D5): its fingerprint (`fingerprint.Fingerprint`: size and modification time, and for an OME-Zarr image the inventory digest), labels, axes, roles, `singleton_roles`, `axes_from` and the path it is read from (so moving or converting the experiment recomputes it), and the settings that change rows (bins, histogram range, robust quantiles, range-check limit). | | Removed keys | `settings.convert_tiffs`, `settings.conversion` and `files[].ome_zarr_path` were written by v0.3 runs that converted TIFF images to OME-Zarr in the output folder (removed in v0.4.0, §10); a manifest that holds them is still read, and they are ignored. The copies such a run left in `/ome_zarr` are not touched. | | Flags | `missing_as_singleton`, `read_in_blocks`, `channel_names_trimmed` and `channels_from_rgb_samples` are JSON booleans (`true`/`false`); any other value (`"false"`, `0`) makes the manifest malformed, never read as true or false, and `write_manifest` refuses to write one (`ValueError`), so a written manifest always reads back. | | `versions` | Versions of `morphobricks-preprocessing`, `morphobricks-normalization` (which computes the histograms and writes the CSV; absent in manifests written before it), `numpy`, `tifffile` and Python. Information only: a different version never causes a rebuild. | @@ -271,19 +266,32 @@ the bricks' tests. | Topic | Rule | |---|---| | Modes | `run_experiment(mode=...)`, CLI `--mode`: `restart` (default) reuses the rows whose `RowKey` is unchanged (§9) and reads new or changed files; `full` recomputes every row and check and reuses no row. Both keep the user's confirmations (design, axis and mask-axis statements, stated calibration, image type) and publish only on success; a failed or stopped run leaves the last good run intact. `force_rebuild` and `--force-rebuild` were removed. | -| Files a run replaces or removes | Only `plane_histograms.csv`, `run_manifest.json`, `experiment_design.csv` (when the run has a design), the report files, `mask_relationships.json` and its own temporary folders; never `calibration.csv` (§9.3). `mask_relationships.json` is removed by a `full` run and by a `restart` whose masks entry (§9) differs from the earlier run's (or when no v2 manifest records it). Source images, masks and anything else are never touched. | +| Files a run replaces or removes | Only `plane_histograms.csv`, `run_manifest.json`, `experiment_design.csv` (when the run has a design), the report files, `mask_relationships.json`, the publication folder `.publish` and the staging folder `.staging` (§9.4); never `calibration.csv` (§9.3). `mask_relationships.json` is removed by a `full` run and by a `restart` whose masks entry (§9) differs from the earlier run's (or when no v2 manifest records it). Source images, masks and anything else are never touched. | | Recorded inputs | One output folder belongs to one experiment. An input not given is taken from the earlier v2 manifest: experiment root, axis mapping with the per-file plain-stack confirmations at their recorded fingerprints (a new or changed plain stack must be confirmed again), exclusions, masks folder and mask-axis statements (statements given are added per mask type), image type, design (`experiment_design.csv` in the output folder), bins (not across pixel types), histogram and intensity settings, `missing_as_singleton`; the stated calibration is the output folder's `calibration.csv` (§9.3). A value given that differs is used and reported as a change with its effect (rows recomputed, or none); for the design, a design given whose answers (repeat group, confirmation, control) differ for folders in both, or `experiment_design.csv` edited since the earlier run wrote it; for the calibration, `calibration.csv` added, edited or removed since the earlier run read it (`calibration`, "no row is affected; the pixel sizes of N images change"). Masks are dropped only with `masks_folder=False` / `--no-masks`, which also drops the mask-axis statements recorded for the masks folder's masks (those of OME-Zarr label images are kept; before PP-10c the recorded statements stayed and the run was refused with `invalid_mask_axes`); exclusions are cleared with `exclude=[]` / `--no-exclude`. A version 1 manifest gives nothing. | | Experiment root | `run_experiment(experiment_root=None, output_folder=...)` and CLI `run --output FOLDER` use the recorded experiment root; without one, `ValueError` (API) or `experiment_not_recorded` (CLI). Another root than the recorded one is a change: every row is read again, and the recorded plain-stack confirmations are not applied there (each plain stack must be confirmed again). | | `masks` command | Uses the recorded inputs the same way (plan §12 question 56): the experiment argument is optional when `--output` holds a v2 manifest recording it (`experiment_not_recorded` otherwise), and the masks folder, mask-axis statements and exclusions are the recorded ones unless given (`--no-exclude` clears the exclusions). A value given that differs is used for that scan and printed as a change; the manifest is not changed (the next run records only what it is given). The pairing with the recorded inputs, the scan and the write under the lock are `run.mask_step.prepare_mask_step(experiment_root, output_folder, exclude=, masks_folder=, mask_axes=)` (a `MaskStep`: the preflight, the output folder, the input changes, the blocking issues, or the `experiment_not_recorded` refusal) and `run.mask_step.describe_masks(step, thresholds=, progress=)` (a `MaskStepResult`: the report and the file written, or the `output_locked` refusal with no mask read and nothing written: the lock is taken before the first mask is read and held to the write; `ValueError` for masks whose pixels cannot be used), shared with the notebook (v0.4.0, PP-10d); the command only prints their results. | | Axis roles | `mapping=None` uses the recorded roles; without any, the run stops with `axes_not_confirmed`. CLI `--axes` is needed for the first run and for plain stacks that are new or changed. | | Lock | `.morphobricks-preprocessing.lock` in the output folder, created with `O_CREAT \| O_EXCL`, JSON `host`, `pid`, `started_at`; held by `run`, by `check` while it writes its report (and the design template), by `masks` from before its first mask read to the write of `mask_relationships.json` (a locked folder is refused before any mask is read), and by `design` and `calibrate` while they change `experiment_design.csv` and `calibration.csv` (CLI `run` also holds it to write the design template after a run stopped on the design), removed on every exit, and removed when the lock file cannot be written. A second run is refused with `output_locked`, naming the holder, and writes nothing. `remove_stale_lock(output)` / `--remove-stale-lock` (every command) removes a lock only on that request, never one of a process still running on the same computer (checked on POSIX). Not written inside the experiment. The lock is not part of the manifest. | -| Report | `discovery_report.json` (report version 1) and `discovery_report.txt`, written on every run that holds the lock, also when it stops: `step` (`run` or `check`), `status` (`stopped` or `written`; `check` is always `stopped`), `provisional` (true for the report written after the header checks, before any pixel is read), `manifest_sha256` (the manifest published with it, `written` only), `experiment_root` and `experiment_root_changed`, `conversion` (for a tree of named-axis copies, §17: `record`, `sha256`, `source_root`; null otherwise), `output_folder`, `masks_folder`, `excluded`, `groups` (labels, position identifiers, image counts), `channels`, `channel_names`, `images` (per image `axes`, `roles`, `axes_from`, `pixel_sizes` by role with unit and source, `header`, `stated` or `pixels`, or `"uncalibrated"` without an X and a Y entry), `calibration` (`file`, `sha256` and the `statements`), `design`, `image_type`, `masks` (per type paired masks and images without one; unpaired and ambiguous masks), `mode`, `reused`, `recomputed` (file: reason), `removed`, `replaced` (file: what happened), `input_changes`, `issues` (code, severity, message, paths, action). For people only: bricks never read it. A published report is replaced atomically with the table and manifest, last. | +| Report | `discovery_report.json` (report version 1) and `discovery_report.txt`, written on every run that holds the lock, also when it stops: `step` (`run` or `check`), `status` (`stopped` or `written`; `check` is always `stopped`), `provisional` (true for the report written after the header checks, before any pixel is read), `manifest_sha256` (the manifest published with it, `written` only), `experiment_root` and `experiment_root_changed`, `conversion` (for a tree of named-axis copies, §17: `record`, `sha256`, `source_root`; null otherwise), `output_folder`, `masks_folder`, `excluded`, `groups` (labels, position identifiers, image counts), `channels`, `channel_names`, `images` (per image `axes`, `roles`, `axes_from`, `pixel_sizes` by role with unit and source, `header`, `stated` or `pixels`, or `"uncalibrated"` without an X and a Y entry), `calibration` (`file`, `sha256` and the `statements`), `design`, `image_type`, `masks` (per type paired masks and images without one; unpaired and ambiguous masks), `mode`, `reused`, `recomputed` (file: reason), `removed`, `resumed` (files whose rows an interrupted run had staged, reused without reading their pixels, §9.4), `staged_discarded` (file: why its staged rows were not reused), `replaced` (file: what happened), `input_changes`, `issues` (code, severity, message, paths, action). For people only: bricks never read it. A published report is part of the run's generation (§9.4). | | `check` | The same step stopped after the report; it writes the report, and the design template when the output folder holds no design (§12; owner decision of 2 November 2026, plan §12 question 57, which amends design v0.4 §1 "it writes only the report"). The notebook's **Check folder** does the same with the same functions (`run.check_step.suggested_review`, `write_check_outputs`; v0.4.0, PP-10): the report, and the template with the design card; nothing when an embedding brick hides the output folder (`folder_step(show_output=False)`). | | Exit status (v0.4.0) | Every command (`check`, `run`, `masks`, `calibrate`, `design`, `convert`) exits with status 1 when it prints an error and 0 otherwise; warnings never change it, and options argparse cannot read exit with 2. `check` exits with 1 on every error it reports: the preflight's, the axis check's, a refused write of its outputs (`output_locked`) and an experiment design it cannot read (`unreadable_experiment_design`; found in the review of #130, it exited with 0) or one with `repeat_group_conflict` or `invalid_experiment_design`, printed after its summary (found in the review of #133: `run` stopped on it, `check` exited with 0); `design_not_confirmed` does not count, since `check` writes the template to confirm. `design` exits with 1 when the design it prints has `repeat_group_conflict` or `invalid_experiment_design`, also after a change it wrote (it exited with 0); `design_not_confirmed` is not printed by `design` and does not count. | | Notebook (v0.4.0, PP-10) | Step 3 offers the mode (`restart`, `full`) with its effect, and lists before the run the inputs that differ from the recorded ones with their effect (`run.planned.planned_changes(given, output_folder, design=, calibration=)`, from `resolve_inputs`, `design_for_run` and `calibration_change`: the changes the run then reports). After a run it names the mode, the files replaced and the input changes, and shows the report; a stopped run says what it wrote (question 55). A refusal `output_locked` (check, run, a design answer or a stated size) shows the holder (computer, process ID, start time) and a **Remove the lock of a stopped run** button: `remove_stale_lock` only on that click, never the lock of a process still running on the same computer; on Google Drive one session per output folder (question 58). | | Notebook masks (v0.4.0, PP-10c) | Step 1 has an optional masks folder; **Check folder** pairs it with `preflight(masks_folder=, mask_axes=)`, the masks folder and statements being those the run would use (`run.planned.planned_masks`, from `resolve_inputs`: given, else recorded). The **Masks** card shows each mask type (`mask_review.mask_type_rows`), the images without one and the masks' issues (`masks.issues`, and `mask_checks.mask_issues` once the roles are confirmed, as `run`). Mask axes are asked per type only where needed (`mask_review.mask_axes_questions`: `mask_not_2d`, `invalid_mask_axes`, `mask_axes_contradict_header`, and `mask_planes_mismatch` of 2-D masks, answered `YX`), with only the statements `masks.stated.mask_roles` accepts for every mask of the type; nothing is preselected or applied until **Check the masks** (`run.preflight.remasked`, which keeps the other answers). The run is given the same `masks_folder` and `mask_axes`; a masks folder not given reuses the recorded one, and **Drop the recorded masks** gives `masks_folder=False`. Step 3 is gated on masks without errors and on a pairing that matches the masks folder and statements chosen. `folder_step(show_output=False)` shows neither and pairs no masks folder. The mask step (`mask_relationships.json`) is step 3's **Describe the masks** (next row). | | Notebook mask step (v0.4.0, PP-10d) | Step 3's **Describe the masks (optional)** card. Its button is enabled when step 1's output folder holds a v2 manifest that recorded masks (`run.mask_step.recorded_masks(output_folder)`, a `RecordedMasks` from the manifest's `experiment_root`, `written_at` and `masks` keys): after a run in this session, or for an earlier run in a reopened notebook. The help line names the run it describes (output folder, when written, experiment folder, masks folder and mask types). The output folder is the one step 1 shows (default: next to the experiment folder); the experiment folder typed in step 1 only locates it, and the recorded one is read. A click runs the `masks` command's step with the run's recorded inputs (`prepare_mask_step(None, output_folder)`, the path the CLI's `masks --output` takes: experiment folder, masks folder, mask-axis statements, exclusions) and the default thresholds (the notebook offers no threshold; the command line's options change them), with a progress bar per image, then `describe_masks`, which reads the masks and writes `mask_relationships.json` holding the lock throughout. The card shows the mask types, the relationships with their counts (`mask_text.mask_type_rows`, `mask_relationship_rows`), masks of the same objects and the hierarchy (`mask_hierarchy_lines`), labelled as suggestions to check, and the path written. A refused step changes nothing: `experiment_not_recorded`, errors found since the run (an experiment or masks folder moved or removed, a mask changed; shown with their messages), no masks, masks that cannot be used (a red card), and `output_locked` (the lock card with **Remove the lock of a stopped run**). A new run clears the card. `folder_step(show_output=False)` hides the card; the pinned embedding API is unchanged (the click handler is private). | +### 9.4 Staging, resume and publication (v0.4.0, PP-12, design v0.4 §1) + +| Topic | Rule | +|---|---| +| Staging | As soon as a run has read one image, that image's rows are staged in the output folder's `.staging` folder: a `.csv` with its rows (the normalization brick's writer, the table's format) and then `.json` (staging version 1) naming the image, its staging key and the rows file's SHA-256. The staging key is the image's row key (§9: fingerprint, labels, axes, roles, `singleton_roles`, `axes_from`, read path, bins and histogram settings) with the pixel type, the table's schema version and the versions of `morphobricks-preprocessing` and `morphobricks-normalization`. A rows file without its record (killed between the two) was never staged. | +| Resume | A run killed part-way (a crash, a kill, a lost Colab session) is resumed by the next run: each image that needs new rows whose staged key is the key it would have now (and whose fingerprint is still that) takes its staged rows without its pixels being read (`RunResult.resumed`, report `resumed`). Any other staged rows are discarded and reported with the reason (report `staged_discarded`): the image changed since its rows were staged; its roles, labels, folder or row settings changed; the experiment's pixel type changed; another version staged them; they could not be read (reported under the image the record names, else the record's file name); the table already holds its rows; the image is no longer analysed; a `full` run (which reuses no staged row). Nothing is reused when identity is uncertain. A run that stops keeps `.staging`, so the next run resumes; a run that publishes removes it after the commit. Every check still runs over the whole experiment (design v0.4 §1). A resumed table is byte-identical to an uninterrupted run's. | +| Source changes during a run (issue #90) | Each image's header is read between two fingerprints that must agree (`preflight(bind_headers=True)`), and that fingerprint is the one its manifest entry records; the image must still have it just after its pixels are read (only then are its rows staged); and every analysed image (reused, resumed or read) is fingerprinted again just before the commit. An image that changed or went missing at any of these points is the error `source_changed_during_run` naming it: nothing is published, the earlier generation is kept, and the rows staged so far stay staged (the changed image's own rows are not staged, or are discarded by the next run). Masks are fingerprinted when the manifest is written, as before. | +| Generation | The run's published files: `plane_histograms.csv`, `experiment_design.csv` (with a design), `run_manifest.json` and the report files, and the files the generation removes (`mask_relationships.json`, §9.2). | +| Publication | (1) The whole new generation is written into `.publish` in the output folder and flushed to disk, then `.publish/publication.json` (publication version 1: each file's SHA-256 and the files removed). (2) **Commit, the one atomic step:** `os.replace` of the new `run_manifest.json` over the old one. Nothing in the output folder changes before it. (3) The other files are moved into place (`os.replace` each), the removed files removed, `.publish` deleted, then `.staging`. | +| Interrupted publication | Whatever next takes the output folder's lock (`run`, `check`, `masks`, `design`, `calibrate`, the notebook's writes) first finishes a committed publication (the manifest in place is the record's: the remaining moves and removals are done) or deletes an uncommitted `.publish` (the earlier generation is then untouched) (`run.generation.finish_publication`). A failure to move a file after the commit (for example a table held open on Windows) raises `PublicationIncompleteError`: the new generation is current, and the next holder of the lock finishes the moves. | +| What readers see | **Through the manifest** (`RunInputs`): exactly one generation, the old or the new one, never a mix: each file the manifest names is read by its SHA-256 from the output folder or, while a publication is unfinished, from `.publish` (`run.generation.published_path`), and the read is repeated if the manifest changed meanwhile (`read_stable`). **A plain reader of `plane_histograms.csv` alone** always reads one whole table of one generation (each move is one rename): the old one until it is moved. A reader that opens several output files without the manifest can pair files of two generations while a publication's step 3 runs or after a kill in it, until the next holder of the lock finishes it: read several files through `RunInputs`, or check `csv_sha256` and `experiment_design_sha256`. | +| Windows | The same protocol. `os.replace` replaces a file atomically on NTFS too, but fails while another program has the file open: at the commit nothing has changed then (the run raises, the earlier generation is kept); after it, `PublicationIncompleteError` as above. Folders are not flushed (Windows cannot open them); the kill tests (SIGKILL at every step) run on POSIX only. | + ### 9.3 Calibration (`calibration.csv`, v0.4.0, design v0.4 §3) | Topic | Rule | @@ -541,5 +549,5 @@ exclude=, missing_as_singleton=, memory_limit=, progress=)` (`conversion`), the | Safe | The copy tree's lock (`.morphobricks-preprocessing.lock`, §9.2's rule; `--remove-stale-lock`) is held throughout; a second conversion is `output_locked`. Each copy is written as `.partial`, the source's fingerprint checked before and after; once every copy is written they are moved into place, then the record is written (atomic, with the file-mode rule of §9). `not_enough_space` is refused before writing. Any failure or interruption removes the `.partial` files of the conversion, the copies already moved into place (each where nothing was) and the folders it made, so the folder is left as it was; a read or write failure is `conversion_failed`, anything else is raised again; `.partial` files of an interrupted conversion are written again. New files get the mode a plain `open()` gives. | | Repeat | A copy already in the tree is reused when its source (fingerprint and axes), roles, copy axes, pixel sizes, unknown sizes (roles and reasons), channel names, significant bits, the copy's fingerprint and the copy's SHA-256 (read whole) are as recorded; a new image is added; when every copy is reused the record is kept as it was (same SHA-256). Anything else (a source changed, other roles, a copy changed, an image no longer in the experiment, another experiment's copy tree, a file at a new image's place that the record does not list) is `copy_differs`: nothing is overwritten. A conversion record that cannot be read, or of version 1, is `conversion_record_unreadable`. | | Conversion record | `.morphobricks-conversion.json` in the copy tree's root (a dot-file, so discovery skips it), record version 2: `source_root`, `mapping`, `missing_as_singleton`, `written_at`, `versions`, and per image `relative_path`, `action` (`converted` or `copied`), `reason` (the axis code, `excluded`, or null), `source` and `copy` fingerprints (`size`, `mtime_ns`), `copy_sha256` (the copy's SHA-256, computed in one pass after it is written; an OME-Zarr copy's is the SHA-256 of its files' relative paths and SHA-256s in path order, its `labels` group left out as for fingerprints: `content_hash`), `source_axes`, `roles`, `copy_axes`, `pixel_sizes` (by role, as §9), `unknown_sizes` (role: why), `channel_names` (as the copy holds them; `[]` for none) and `significant_bits` (or null). Sources keep the fingerprint rule (design v0.4 §1); the copies' SHA-256 is the owner's decision of 2 November 2026 (plan §12 question 59 (16)). Version 2 (PP-11c) added `copy_sha256`, `channel_names` and `significant_bits`; a version 1 record (v0.4.0rc3 or earlier) is refused everywhere it is read (`convert`, the run's `review_conversion`, `RunInputs`) with a message to convert the originals again into a new folder: its copies cannot be checked. `read_conversion_record(folder)` reads it with its SHA-256. | -| The run on the copies | The user runs preprocessing on the copy tree with the same output folder (`--output`): the recorded experiment root becomes the copy tree and the report says so (`experiment_root_changed`, and `conversion` with the record, its SHA-256 and `source_root`; the text says "named-axis copies of ..."); every row is recomputed (the row key includes the read path, §9); the design, image type and calibration are kept (same relative paths). The manifest records `conversion_record_sha256`. The run and `check` warn when a source changed or went missing after the conversion (`conversion_source_changed`) and when a copy changed (its fingerprint or its SHA-256: each copy is read whole, so `check` and `run` on a copy tree cost one more read of the copies), went missing or was added since (`copy_changed_after_conversion`; the run goes on, and its message says that `RunInputs` will refuse the run for a changed or added copy); an unreadable record stops the run (`conversion_record_unreadable`). `RunInputs` reads the copies like any images, and refuses a run whose conversion record changed or went missing since (`conversion_record_sha256`), is of version 1, or whose recorded images' copies no longer have the recorded fingerprint and SHA-256 (§9.1). | +| The run on the copies | The user runs preprocessing on the copy tree with the same output folder (`--output`): the recorded experiment root becomes the copy tree and the report says so (`experiment_root_changed`, and `conversion` with the record, its SHA-256 and `source_root`; the text says "named-axis copies of ..."); every row is recomputed (the row key includes the read path, §9); the design, image type and calibration are kept (same relative paths). The manifest records `conversion_record_sha256`. The run and `check` warn when a source changed or went missing after the conversion (`conversion_source_changed`) and refuse a copy that changed (its fingerprint or its SHA-256: each copy is read whole, so `check` and `run` on a copy tree cost one more read of the copies), went missing or was added since (`copy_changed_after_conversion`: an error since PP-12, owner decision of 2 November 2026; the run stops after the header checks, before any pixel is read, writing only its report, since `RunInputs` would refuse the run; the message says to convert the originals again into a new folder); an unreadable record stops the run (`conversion_record_unreadable`). `RunInputs` reads the copies like any images, and refuses a run whose conversion record changed or went missing since (`conversion_record_sha256`), is of version 1, or whose recorded images' copies no longer have the recorded fingerprint and SHA-256 (§9.1). | | Notebook (PP-11b) | Step 2's **Make named-axis copies (optional)** card, under the axis check, shown only when it reports images with `axes_yx_swapped`, `axes_contradict_header`, `axes_x_before_y` or `axes_not_named` for the roles being edited (design v0.4 §5: offered only for files with errors), and not with `folder_step(show_output=False)`. The roles are the user's: those confirmed in step 2, or the roles shown once the preview is checked and the card's own question is answered; any change of a role or of the missing-axes answer takes it back. A required folder for the copies (Browse…, or pasted) checked with the command's rule (`copy_folder_issues`); `preview_conversion` (`conversion_plan`, headers only, writes nothing) gives what will be converted and copied byte for byte (the command's `plan_conversion`), the space needed and the free space; then `convert_experiment` with a progress bar, under the copy tree's lock. Refusals use the existing cards (the lock card with **Remove the lock of a stopped run**, the rule of `--remove-stale-lock`). After the copies: the record's path, and **Check the copies in step 1**, which sets the experiment folder to the copy tree, keeps the output folder and checks again; the card says that the missing-axes answer must be given again and lists the copies whose relabelled sizes are unknown (`ConversionResult.unknown_sizes`), to state under **Pixel sizes**. | diff --git a/docs/development.md b/docs/development.md index 9819f314..bc419921 100644 --- a/docs/development.md +++ b/docs/development.md @@ -87,11 +87,13 @@ is pinned to the release in `RELEASE_TAG` (plan §12 question 58); - Floats are written at full precision (`repr`); files are UTF-8. - Outputs are written to a temporary file and replace the previous version only after a complete successful write. -- Experiment runs stage both the CSV and manifest (and any OME-Zarr copies) before - publishing either, back up earlier outputs, and restore their previous state if - publication raises. Preserve - backups if recovery fails. The CSV hash check still detects forced termination between - replacements; run only one writer per output folder. +- Experiment runs publish their outputs as one generation (contract §9.4, PP-12): the + whole generation is written into `.publish`, then the manifest's `os.replace` commits + it, then the other files are moved in. Whatever takes the output folder's lock first + finishes or discards an interrupted publication (`run.generation.finish_publication`); + readers go through the manifest (`published_path`, `read_stable`). Each image's rows are + staged in `.staging` as soon as it is read, so an interrupted run resumes. Run only one + writer per output folder (the lock). ## Tests diff --git a/docs/plan.md b/docs/plan.md index fcc23828..bfe88331 100644 --- a/docs/plan.md +++ b/docs/plan.md @@ -568,6 +568,40 @@ API, CLI). The notebook and CLI only call the API. `invalid_physical_calibration`, a stated one is refused, and an infinite size is never the same as another; `check` and `design` exit with status 1 whenever they print an error. +- **PP-12 (staging and resume)** *(implemented, PR #132; design v0.4 §1, §8; issues #86, + #90; question 69; contract §9.4)*: + - Move only, first: `source_fingerprint` and `LABELS_GROUP` from `manifest` to + `inventory` (io; `manifest` re-exports `source_fingerprint`). + - #86: an OME-Zarr image's fingerprint adds the SHA-256 of its inventory (each file's + relative path, size and `mtime_ns`, sorted by path; still metadata only): + `Fingerprint.inventory`, `FileEntry.inventory_sha256` (OME-Zarr only), kept for label + images and copies too; `check_sources` compares it; a record without it counts as + changed. A folder inside the image that cannot be listed raises instead of being + skipped. + - Staging (`run.staging`) and resume (`run.resume`): each image's rows staged in + `.staging` as soon as it is read; the next run reuses them only when the staging key + (row key, pixel type, schema version, the versions of preprocessing and normalization) + and the fingerprint still match, and reports `resumed` and `staged_discarded` (reason + per file); a stopped run keeps `.staging`, a published run removes it; `full` reuses + none. + - Crash-consistent publication (`run.generation`, owner decision of 2 November 2026, + question 69): the generation in `.publish` with a publication record, committed by + the manifest's `os.replace`, then moved in; the lock finishes or discards an + interrupted publication; `RunInputs` reads by SHA-256 (`published_path`) and again if + the manifest changed (`read_stable`). The backup-and-restore protocol is gone. + - #90: headers bound to fingerprints in the run (`bind_headers=True`), the entry records + that fingerprint, the image is fingerprinted after its pixels are read (before its rows + are staged) and every image before the commit: `source_changed_during_run` (error). + - Owner decision: `copy_changed_after_conversion` is an error that stops the run before + it writes (§17). + - Not in this PR: a v0.4.0rc3 run on OME-Zarr images must be run again (its entries + have no inventory: recomputed, and `RunInputs` refuses them). + - **Follow-ups (review of PR #132):** (a) masks are fingerprinted when the manifest is + written, as before, not bound to the fingerprint taken with their headers during the + run (owner and coordinator: a follow-up, not a blocker); (b) a run killed while an + atomic write's temporary file is open leaves that hidden `...tmp` file in + the output folder (on main too, for the provisional report); the lock could remove + them. - **Release v0.4.0 (PP-13) checklist**, for every tag of v0.4.0 including release candidates, in the release PR before the tag is pushed: - set `version` in `pyproject.toml`; @@ -1681,3 +1715,33 @@ Raised during review; to be decided before the milestones that need them: contradicted roles. Images without confirmed roles that the header names are compared by their header, as before. The notebook's axis card still shows only the axis review's own issues; the corrected warnings appear in the run's result. Contract §11.* +69. How does a run resume after an interruption, and how is its publication made + crash-consistent (PP-12, design v0.4 §1)? *Decided (owner, 2 November 2026; contract + §9.4):* (1) **Resume:** each image's rows are staged as soon as it is read; staged rows + are reused only when their row key, the pixel type, the schema version and the + versions of the two packages that compute rows are unchanged and the image still has + its fingerprint; anything else is discarded and reported, never reused. The staging + folder is kept until a run publishes (design v0.4 §1 says it is removed "at the start + of the next run"; keeping it until publication means a second interruption loses + nothing; accepted by the owner and coordinator on 2 November 2026 on the condition, + checked in review, that every run re-validates every staged row at its start). (2) + **Publication:** the owner asked that any reader see the old or the new generation, + never a mix, after a kill between any two steps. Chosen: manifest-last commit, the + simplest design that keeps the documented file names as plain files on every system: + the whole generation in `.publish`, a publication record, then one `os.replace` of + `run_manifest.json` (the commit), then the moves; the next holder of the lock finishes + or discards it; readers through the manifest read each file by its SHA-256 (from + `.publish` while unfinished); a plain reader of `plane_histograms.csv` always reads one + whole table (the old one until moved). Rejected: a generation folder with an atomic + symbolic link, which would make every reader consistent but needs symbolic links + (Windows without Developer Mode, Google Drive in Colab and copied or zipped output + folders do not keep them); generation-named files with the manifest as pointer, which + changes the documented `plane_histograms.csv` path or doubles the disk space. A plain + reader that opens several files without the manifest can pair two generations during + or after an interrupted move; it is told to read through `RunInputs`. (3) **#90:** + headers bound to fingerprints, each image fingerprinted after its pixels are read and + every image before the commit; a change is `source_changed_during_run` and nothing is + published. (4) **#86:** the OME-Zarr inventory digest (each file's path, size and + time), metadata only as design v0.4 §1 decides; content hashing stays the owner's + call. (5) **`copy_changed_after_conversion`** is an error that stops the run before it + writes (owner, 2 November 2026): the bricks would refuse the run anyway. diff --git a/docs/user_guide.md b/docs/user_guide.md index cfb7cf1a..94e6e1d7 100644 --- a/docs/user_guide.md +++ b/docs/user_guide.md @@ -768,8 +768,10 @@ record, **.morphobricks-conversion.json** in the folder of copies, lists every i its original's and its copy's size and modification time, the copy's SHA-256 (a checksum of its content), the roles, the pixel sizes, the channel names and the significant bits; the run records the record's SHA-256 and warns when an original changes after the conversion -(`conversion_source_changed`) or a copy is changed (`copy_changed_after_conversion`, also a -change that keeps the copy's size and time). Checking the copies reads each of them whole, +(`conversion_source_changed`). A copy that changed (also a change that keeps the copy's size +and time), went missing or was added since the conversion stops the run before anything but +the report is written (`copy_changed_after_conversion`, an error): later bricks would refuse +such a run, so convert the originals again into a new folder and run on that one. Checking the copies reads each of them whole, so `check` and `run` on a folder of copies, and opening their run, take about one read of the copies longer. An original that changes while `convert` reads its metadata is refused (`source_changed_during_check`); convert again once nothing writes into the experiment. @@ -870,11 +872,28 @@ The default output folder is next to the experiment folder: When masks are given, `run_manifest.json` also lists every image's masks (`masks`), with each mask's axes and size (for example `TYX` and 50 x 512 x 512 for a mask per time point). -The table and manifest are both prepared before the earlier files are replaced. If a -replacement fails, the earlier files are restored. This needs temporary disk space for -the new outputs and copies of the earlier pair. If restoration also fails (for example, -another program locks an output file), the error names a backup folder: close programs -using the output files and restore those backups before running again. +**Interrupted runs resume.** As soon as an image is read, its rows are kept in the +hidden folder `.staging` in the output folder. If the run is interrupted (a crash, a +stopped kernel, a lost Colab session, a computer switched off), run again with the same +settings: the images already read are not read again, as long as they are unchanged (same +size, time and, for OME-Zarr, the same files) and the settings that change rows are the +same. The report lists them as `resumed`, and lists any rows it did not reuse with the +reason (`staged_discarded`). A `full` run reuses none. After a killed run, remove its lock +first (`--remove-stale-lock`). Once a run has written its results, `.staging` is deleted. + +**Results are switched in one step.** The new table, design, manifest and report are first +written into the hidden folder `.publish`; replacing `run_manifest.json` then makes them +the current results in one step, and the other files are moved into place. A run stopped +before that step leaves the earlier results exactly as they were; one stopped after it is +finished by the next run (or `check`, `masks`, `design`, `calibrate`). Later bricks always +read one whole set of results, old or new. If a file cannot be moved into place (for +example because another program has the table open on Windows), the run says so: close +that program and run again. This needs temporary disk space for the new outputs. + +**Images that change during the run.** If an image changes or disappears while the run +checks or reads it (an acquisition or a copy still writing into the experiment), the run +stops with `source_changed_during_run` and publishes nothing. Wait until nothing writes to +the experiment, then run again: the images already read resume. Every output file gets the permissions a file you save yourself would get (your umask; usually readable by everyone, writable by you), so colleagues can open the results in a @@ -971,7 +990,8 @@ continues. | `not_enough_space` | Error | `convert`: the copies would not fit on the drive. Free space or choose another drive. | | `conversion_record_unreadable` | Error | The conversion record of a folder of named-axis copies (**.morphobricks-conversion.json**) cannot be read, or is of version 1 (made by v0.4.0rc3 or earlier, without the copies' SHA-256), so `run` stops and `convert` refuses the folder. Convert again into a new folder. | | `conversion_source_changed` | Warning | An original changed or went missing after its named-axis copy was made; the copies hold the images as converted. If the originals were corrected, convert again into a new folder. | -| `copy_changed_after_conversion` | Warning | An image of the folder of named-axis copies changed (its size, time or SHA-256), went missing or was added after the conversion, so the conversion record does not describe it. The run goes on, but the bricks (`RunInputs`) refuse a run on such copies: convert the originals again into a new folder. | +| `copy_changed_after_conversion` | Error | An image of the folder of named-axis copies changed (its size, time or SHA-256), went missing or was added after the conversion, so the conversion record does not describe it, and the bricks (`RunInputs`) would refuse a run on it. The run stops before it writes anything but the report: convert the originals again into a new folder, and run preprocessing on that one (same output folder). | +| `source_changed_during_run` | Error | An image changed or went missing while the run was checking or reading it, so its rows would not describe it. Nothing is published. Wait until nothing writes to the experiment (an acquisition or a copy still running), then run again: the images already read resume from their staged rows. | | `design_unreadable` | Error | The `experiment_design.csv` an earlier run wrote cannot be read. Correct it, or delete it to confirm the design again. | | `no_usable_planes` | Error | No plane has finite pixel values. Check the images. | | `tiff_outside_leaf` | Error | Images sit in a folder that also holds subfolders, so they have no condition or repeat of their own. A folder holds either images or subfolders, never both. Move the images into a folder of their own, or out of the experiment folder. | diff --git a/pyproject.toml b/pyproject.toml index 60761dda..8242cfc7 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -179,10 +179,12 @@ baseline = "tools/code_rules_baseline.json" "morphobricks_preprocessing.image_type" = "pure" "morphobricks_preprocessing.intensity" = "io" "morphobricks_preprocessing.manifest" = "io" +"morphobricks_preprocessing.inventory" = "io" "morphobricks_preprocessing.fingerprint" = "io" "morphobricks_preprocessing.narrow_ranges" = "pure" "morphobricks_preprocessing.recorded_sizes" = "pure" "morphobricks_preprocessing.run_inputs" = "io" +"morphobricks_preprocessing.run_inputs_text" = "pure" "morphobricks_preprocessing.unrecorded" = "io" "morphobricks_preprocessing.testing" = "orchestration" "morphobricks_preprocessing.mask_scan" = "io" @@ -223,6 +225,9 @@ baseline = "tools/code_rules_baseline.json" "morphobricks_preprocessing.run.steps" = "orchestration" "morphobricks_preprocessing.run.preflight" = "io" "morphobricks_preprocessing.run.publish" = "io" +"morphobricks_preprocessing.run.generation" = "io" +"morphobricks_preprocessing.run.staging" = "io" +"morphobricks_preprocessing.run.resume" = "io" "morphobricks_preprocessing.run.result" = "pure" "morphobricks_preprocessing.run.reuse" = "io" "morphobricks_preprocessing.run.rows" = "io" diff --git a/src/morphobricks_preprocessing/content_hash.py b/src/morphobricks_preprocessing/content_hash.py index 6d35629a..d61685e8 100644 --- a/src/morphobricks_preprocessing/content_hash.py +++ b/src/morphobricks_preprocessing/content_hash.py @@ -7,7 +7,7 @@ §1). An OME-Zarr copy is hashed as the SHA-256 of its files' relative paths and SHA-256s, in -path order. Its ``labels`` group is left out, as :func:`.manifest.source_fingerprint` +path order. Its ``labels`` group is left out, as :func:`.inventory.source_fingerprint` leaves it out: label images are masks, recorded and checked one by one by the run, and a brick may add a label image to an OME-Zarr image without changing the image. A folder holding a link to a folder is not hashed (:class:`OSError`): the files behind the link @@ -20,7 +20,8 @@ import os from pathlib import Path -from .manifest import LABELS_GROUP, file_sha256 +from .inventory import LABELS_GROUP +from .manifest import file_sha256 def content_sha256(path: str | os.PathLike) -> str: diff --git a/src/morphobricks_preprocessing/conversion_record.py b/src/morphobricks_preprocessing/conversion_record.py index f86d8cc9..0ea965cb 100644 --- a/src/morphobricks_preprocessing/conversion_record.py +++ b/src/morphobricks_preprocessing/conversion_record.py @@ -308,8 +308,8 @@ class ConversionReview: source_root The experiment folder the copies were made from, or None. issues - ``conversion_source_changed`` and ``copy_changed_after_conversion`` (warnings), or - ``conversion_record_unreadable`` (an error). + ``conversion_source_changed`` (a warning), ``copy_changed_after_conversion`` and + ``conversion_record_unreadable`` (errors: the run stops before it writes). record The record's file name, in the experiment folder. """ @@ -408,18 +408,23 @@ def _source_issue(paths: list[str], source_root: str) -> list[ValidationIssue]: def _copy_issue(paths: list[str]) -> list[ValidationIssue]: - """Warn about copies changed, gone or added since the conversion.""" + """Refuse copies changed, gone or added since the conversion (owner, 2 November 2026). + + The bricks would refuse a run on them (``RunInputs`` checks every recorded copy against + the record), so the run stops before it writes rather than publish one they refuse. + """ if not paths: return [] return [ - warning( + error( "copy_changed_after_conversion", f"{plural(len(paths), 'image')} of this folder of named-axis copies changed, went " "missing or were added after the conversion, so the conversion record does not " - "describe them. The bricks will refuse a run on them (RunInputs checks every " - "copy the run records against the record).", + "describe them, and the bricks would refuse a run on them. No image was read and " + "no table or manifest was written.", paths, - "Convert the originals again into a new folder and run preprocessing on that one.", + "Convert the originals again into a new folder, and run preprocessing on that " + "one (with the same output folder).", ) ] @@ -437,7 +442,7 @@ def _unreadable(exc: ConversionRecordError) -> ValidationIssue: def _fingerprint_data(fingerprint: Fingerprint) -> dict: """Return a fingerprint as JSON-compatible data.""" - return {"size": fingerprint.size, "mtime_ns": fingerprint.mtime_ns} + return fingerprint.to_data() def _choice(value: object, allowed: tuple[str, ...], name: str) -> str: diff --git a/src/morphobricks_preprocessing/discovery_report.py b/src/morphobricks_preprocessing/discovery_report.py index efaecf9c..1ef09f6a 100644 --- a/src/morphobricks_preprocessing/discovery_report.py +++ b/src/morphobricks_preprocessing/discovery_report.py @@ -66,6 +66,10 @@ class RunOutcome: ``{output file: what happened}`` for the earlier output files replaced or removed. changes Inputs given that differ from the recorded ones (:class:`.run.recorded.InputChange`). + resumed + Files whose rows an interrupted run had staged, reused (design v0.4 §1). + staged_discarded + ``{file: why}`` for the staged rows of an interrupted run that were not reused. """ step: str = "run" @@ -76,6 +80,8 @@ class RunOutcome: removed: tuple[str, ...] = () replaced: Mapping[str, str] = field(default_factory=dict) changes: tuple[InputChange, ...] = () + resumed: tuple[str, ...] = () + staged_discarded: Mapping[str, str] = field(default_factory=dict) @dataclass(frozen=True) @@ -120,6 +126,8 @@ def to_dict(self) -> dict: "reused": list(outcome.reused), "recomputed": dict(outcome.recomputed), "removed": list(outcome.removed), + "resumed": list(outcome.resumed), + "staged_discarded": dict(outcome.staged_discarded), "replaced": dict(outcome.replaced), "input_changes": [change.to_dict() for change in outcome.changes], "issues": [_issue_dict(issue) for issue in self.issues], @@ -448,6 +456,16 @@ def _outcome_lines(outcome: RunOutcome) -> list[str]: ] reasons = Counter(outcome.recomputed.values()) lines.extend(f" {count} recomputed: {reason}" for reason, count in reasons.most_common()) + if outcome.resumed: + lines.append( + f" Resumed: {plural(len(outcome.resumed), 'image')} whose rows an interrupted " + "run had staged" + ) + discarded = Counter(outcome.staged_discarded.values()) + lines.extend( + f" {count} staged {'file' if count == 1 else 'files'} not reused: {why}" + for why, count in discarded.most_common() + ) lines.extend(f" {name}: {what}" for name, what in sorted(outcome.replaced.items())) for change in outcome.changes: lines.append( diff --git a/src/morphobricks_preprocessing/fingerprint.py b/src/morphobricks_preprocessing/fingerprint.py index 616bbaad..9b5cc903 100644 --- a/src/morphobricks_preprocessing/fingerprint.py +++ b/src/morphobricks_preprocessing/fingerprint.py @@ -4,8 +4,8 @@ when it decides which rows to reuse, and by the bricks' checks: - :class:`Fingerprint`: a file's size and modification time (for an OME-Zarr folder, the - total size of its files and the newest time), as :func:`.manifest.source_fingerprint` - gives them; + total size of its files, the newest time and the digest of its inventory: every file's + path, size and time, issue #86), as :func:`.inventory.source_signature` gives them; - :class:`RowKey`: everything a row depends on. A row is reused only when its file is unchanged and its roles, ``axes_from`` and every setting that changes rows are unchanged (bins, histogram range, robust quantiles, range-check limit, the path it is read from). @@ -20,7 +20,8 @@ from typing import TYPE_CHECKING from .axis_check import ConfirmedAxes -from .manifest import FileEntry, RunSettings, source_fingerprint +from .inventory import INVENTORY_KEY, source_signature +from .manifest import FileEntry, RunSettings if TYPE_CHECKING: from collections.abc import Iterable @@ -38,10 +39,15 @@ class Fingerprint: Size in bytes; for an OME-Zarr folder, the total size of its files. mtime_ns Modification time in nanoseconds; for an OME-Zarr folder, its newest file's. + inventory + For an OME-Zarr folder, the SHA-256 of its inventory (every file's relative path, + size and modification time), so any chunk added, removed or changed changes the + fingerprint (issue #86); None for a file. """ size: int mtime_ns: int + inventory: str | None = None @classmethod def of(cls, path: str | os.PathLike) -> Fingerprint: @@ -52,15 +58,36 @@ def of(cls, path: str | os.PathLike) -> Fingerprint: FileNotFoundError If the file does not exist. """ - size, mtime_ns = source_fingerprint(path) - return cls(size, mtime_ns) + return cls(*source_signature(path)) @classmethod def recorded(cls, entry: FileEntry | Mapping) -> Fingerprint: - """Return the fingerprint recorded for a file (a manifest entry or its data).""" + """Return the fingerprint recorded for a file (a manifest entry or its data). + + A record without an inventory (a file, or a record written before it was kept) + has None, so an OME-Zarr folder recorded without one never equals it now. + """ if isinstance(entry, Mapping): - return cls(int(entry["size"]), int(entry["mtime_ns"])) - return cls(entry.size, entry.mtime_ns) + inventory = entry.get(INVENTORY_KEY) + return cls( + int(entry["size"]), + int(entry["mtime_ns"]), + None if inventory is None else str(inventory), + ) + return cls(entry.size, entry.mtime_ns, entry.inventory_sha256) + + def to_data(self) -> dict: + """Return the fingerprint as JSON data: ``size``, ``mtime_ns`` (and the inventory). + + Returns + ------- + dict + The inventory digest is under ``inventory_sha256``, only for a folder. + """ + data = {"size": self.size, "mtime_ns": self.mtime_ns} + if self.inventory is not None: + data[INVENTORY_KEY] = self.inventory + return data @dataclass(frozen=True) diff --git a/src/morphobricks_preprocessing/inventory.py b/src/morphobricks_preprocessing/inventory.py new file mode 100644 index 00000000..1c7511ca --- /dev/null +++ b/src/morphobricks_preprocessing/inventory.py @@ -0,0 +1,108 @@ +"""A source image's fingerprint as it is on disk now (contract §9, design v0.4 §1). + +The fingerprint is metadata only, never the pixels (design v0.4 §1): + +- a TIFF file gives its own size and modification time; +- an OME-Zarr folder gives the total size of its files, the time of the newest one, and + the SHA-256 of its **inventory**: every file's relative path, size and modification + time, sorted by path (issue #86). Adding, removing, renaming, resizing or touching any + chunk or metadata file changes the inventory, even when the total size and the newest + time stay the same. + +What no metadata can see is a file rewritten with the same size whose modification time +is then set back to its earlier value (as for a TIFF file): only hashing the content would, +and design v0.4 §1 keeps the fingerprint metadata only. + +Moved from :mod:`.manifest` (PP-12), which re-exports :func:`source_fingerprint`. +""" + +from __future__ import annotations + +import hashlib +import os +from pathlib import Path + +LABELS_GROUP = "labels" +"""An OME-Zarr image's group of label images (masks): outside the image's fingerprint.""" + +INVENTORY_KEY = "inventory_sha256" +"""The key of an OME-Zarr folder's inventory digest in the manifest and other records.""" + + +def source_signature(path: str | os.PathLike) -> tuple[int, int, str | None]: + """Return the size, modification time and inventory digest of an image. + + Parameters + ---------- + path + A TIFF file or an OME-Zarr folder (the ``labels`` group of the folder's root is + left out: label images are masks, fingerprinted one by one, design v0.4 §2). + + Returns + ------- + tuple + ``(size, mtime_ns, inventory)``: for a file its own size and time, and None; for + a folder the total size of its files, the newest time (the folder's own + included) and the SHA-256 of its inventory (one line per file: the path relative + to the folder with ``/``, its size and its ``mtime_ns``, sorted by path). + + Raises + ------ + OSError + If the image does not exist (``FileNotFoundError``), a folder inside it cannot + be listed, or links reach one folder twice: an inventory is never taken with a + part left out. Links to folders inside the image are followed. + """ + path = Path(path) + if not path.is_dir(): + stat = path.stat() + return stat.st_size, stat.st_mtime_ns, None + found = [] + newest = path.stat().st_mtime_ns + seen: set[str] = set() + # A link to a folder is followed, as the image's reader follows it: its files are part + # of the image. A folder reached twice (a loop of links) is refused. + for folder, subfolders, names in os.walk(path, onerror=_raise, followlinks=True): + here = Path(folder) + real = os.path.realpath(here) + if real in seen: + raise OSError(f"{path} reaches the folder {real} twice through links") + seen.add(real) + if here == path and LABELS_GROUP in subfolders: + subfolders.remove(LABELS_GROUP) + relative = here.relative_to(path).as_posix() + for name in names: + stat = (here / name).stat() + found.append((name if relative == "." else f"{relative}/{name}", stat)) + newest = max(newest, stat.st_mtime_ns) + found.sort(key=lambda item: item[0]) + digest = hashlib.sha256() + for name, stat in found: + digest.update(os.fsencode(name) + f"\0{stat.st_size}\0{stat.st_mtime_ns}\n".encode()) + return sum(stat.st_size for _, stat in found), newest, digest.hexdigest() + + +def source_fingerprint(path: str | os.PathLike) -> tuple[int, int]: + """Return the size and modification time of an image, without its inventory. + + Kept for callers that show or compare only these two numbers; whether an image + changed is decided with the inventory too (:func:`source_signature`, + :class:`.fingerprint.Fingerprint`). + + Parameters + ---------- + path + A TIFF file or an OME-Zarr folder. + + Returns + ------- + tuple + ``(size, mtime_ns)`` as :func:`source_signature` gives them. + """ + size, mtime_ns, _ = source_signature(path) + return size, mtime_ns + + +def _raise(error: OSError) -> None: + """Raise a listing error of :func:`os.walk`, which would otherwise skip the folder.""" + raise error diff --git a/src/morphobricks_preprocessing/log_target.py b/src/morphobricks_preprocessing/log_target.py index fb925ec8..32a64b71 100644 --- a/src/morphobricks_preprocessing/log_target.py +++ b/src/morphobricks_preprocessing/log_target.py @@ -35,10 +35,11 @@ from .links import is_inside from .manifest import MANIFEST_FILENAME from .mask_scan import MASK_REPORT_FILENAME +from .run.generation import PUBLISH_FOLDER from .run.lock import LOCK_FILENAME -from .run.publish import STAGING_PREFIX from .run.recorded import recorded_masks_folder, usable from .run.reuse import read_previous +from .run.staging import STAGING_FOLDER PROTECTED_OUTPUTS = frozenset( { @@ -51,6 +52,8 @@ MASK_REPORT_FILENAME, LOCK_FILENAME, RECORD_FILENAME, + PUBLISH_FOLDER, + STAGING_FOLDER, } ) """Names in an output folder a log may not be written to, or inside.""" @@ -86,7 +89,7 @@ def log_refusal( parts = None if output is None else _parts_below(target, Path(output)) if parts is not None: first = parts[0].casefold() if parts else "" - if not parts or first in _PROTECTED_FOLDED or first.startswith(STAGING_PREFIX): + if not parts or first in _PROTECTED_FOLDED: return _refused(log, f"it would write over the run's {parts[0] if parts else 'folder'}") if target.is_dir(): return _refused(log, "it is a folder") diff --git a/src/morphobricks_preprocessing/manifest.py b/src/morphobricks_preprocessing/manifest.py index e2fe4091..cb38bc8d 100644 --- a/src/morphobricks_preprocessing/manifest.py +++ b/src/morphobricks_preprocessing/manifest.py @@ -37,6 +37,8 @@ from ._io import write_text_atomic from .axes import AxisMapping from .image_names import position_id +from .inventory import INVENTORY_KEY, source_signature +from .inventory import source_fingerprint as source_fingerprint # re-exported (moved, PP-12) from .narrow_ranges import NarrowRange from .recorded_sizes import RecordedSize, sizes_from_data, sizes_to_data @@ -47,9 +49,6 @@ LEGACY_MANIFEST_VERSION = "1" """Manifests written before v0.4.0: read for reporting, never trusted (no row is reused).""" -LABELS_GROUP = "labels" -"""An OME-Zarr image's group of label images (masks): outside the image's fingerprint.""" - class ManifestFormatError(ValueError): """Raised when a file is not a run manifest this version can read.""" @@ -136,6 +135,11 @@ class FileEntry: pixel_sizes The size of one step along X, Y, Z and T, each with its unit and source (:class:`.recorded_sizes.RecordedSize`); empty in older manifests. + inventory_sha256 + For an OME-Zarr folder, the SHA-256 of its inventory (each file's relative path, + size and modification time, :func:`.inventory.source_signature`); None for a + TIFF file, and in manifests written before it was recorded (such an OME-Zarr + image counts as changed: its fingerprint is not known to be complete). """ relative_path: str @@ -153,6 +157,7 @@ class FileEntry: roles: tuple[str | None, ...] | None = None axes_from: str | None = None pixel_sizes: tuple[RecordedSize, ...] = () + inventory_sha256: str | None = None def same_source(self, other: FileEntry) -> bool: """Return True if ``other`` describes the same unchanged file, read the same way.""" @@ -160,6 +165,7 @@ def same_source(self, other: FileEntry) -> bool: self.relative_path == other.relative_path and self.size == other.size and self.mtime_ns == other.mtime_ns + and self.inventory_sha256 == other.inventory_sha256 and self.labels == other.labels and self.axes == other.axes and self.singleton_roles == other.singleton_roles @@ -386,46 +392,55 @@ def file_entry_for( # noqa: PLR0913 channel_names_trimmed: bool = False, ) -> FileEntry: """Describe a source image as it is on disk now, with its position identifier.""" - size, mtime_ns = source_fingerprint(Path(root) / relative_path) - return FileEntry( - relative_path=relative_path, - size=size, - mtime_ns=mtime_ns, - labels=tuple(labels), - axes=mapping.axes, - singleton_roles=mapping.singleton_roles, + return file_entry_with( + source_signature(Path(root) / relative_path), + relative_path, + labels, + mapping, plane_count=plane_count, channel_names=tuple(channel_names), significant_bits=significant_bits, read_in_blocks=read_in_blocks, - position_id=position_id(relative_path), channel_names_trimmed=channel_names_trimmed, - roles=tuple(mapping.roles), ) -def source_fingerprint(path: str | os.PathLike) -> tuple[int, int]: - """Size and modification time used to tell whether an image changed. +def file_entry_with( + signature: tuple[int, int, str | None], + relative_path: str, + labels: tuple[str, str, str], + mapping: AxisMapping, + **header: object, +) -> FileEntry: + """Describe a source image with a fingerprint already taken, as :func:`file_entry_for`. - A TIFF file gives its own size and time. An OME-Zarr folder gives the total - size of its files and the time of the newest one, so rewriting any chunk or - metadata file counts as a change. Its ``labels`` group is left out: label - images are masks, fingerprinted one by one (design v0.4 §2), so adding or - changing one does not change the image. + Parameters + ---------- + signature + ``(size, mtime_ns, inventory)`` (:func:`.inventory.source_signature`), for example + the one taken when its header was read. + relative_path, labels, mapping + As :func:`file_entry_for`. + **header + The keyword arguments of :func:`file_entry_for` (channel names, bits, ...). + + Returns + ------- + FileEntry """ - path = Path(path) - if not path.is_dir(): - stat = path.stat() - return stat.st_size, stat.st_mtime_ns - size, newest = 0, path.stat().st_mtime_ns - for folder, subfolders, names in os.walk(path): - if Path(folder) == path and LABELS_GROUP in subfolders: - subfolders.remove(LABELS_GROUP) - for name in names: - stat = (Path(folder) / name).stat() - size += stat.st_size - newest = max(newest, stat.st_mtime_ns) - return size, newest + size, mtime_ns, inventory = signature + return FileEntry( + relative_path=relative_path, + size=size, + mtime_ns=mtime_ns, + labels=tuple(labels), + axes=mapping.axes, + singleton_roles=mapping.singleton_roles, + position_id=position_id(relative_path), + roles=tuple(mapping.roles), + inventory_sha256=inventory, + **header, + ) def package_versions() -> dict[str, str]: @@ -497,7 +512,7 @@ def _file_dict(entry: FileEntry, *, strict: bool = True) -> dict: """ if strict and (entry.roles is None or entry.axes_from not in AXES_FROM): raise ValueError(f"the entry of {entry.relative_path!r} has no roles or axes_from") - return { + data = { "relative_path": entry.relative_path, "size": entry.size, "mtime_ns": entry.mtime_ns, @@ -516,6 +531,9 @@ def _file_dict(entry: FileEntry, *, strict: bool = True) -> dict: "axes_from": entry.axes_from, "pixel_sizes": sizes_to_data(entry.pixel_sizes), } + if entry.inventory_sha256 is not None: + data[INVENTORY_KEY] = entry.inventory_sha256 + return data def _file_entry(data: dict, *, strict: bool = True) -> FileEntry: @@ -543,6 +561,7 @@ def _file_entry(data: dict, *, strict: bool = True) -> FileEntry: roles=_roles(data.get("roles"), str(data["axes"])), axes_from=_text(data.get("axes_from")), pixel_sizes=sizes_from_data(data.get("pixel_sizes")), + inventory_sha256=_text(data.get(INVENTORY_KEY)), ) diff --git a/src/morphobricks_preprocessing/mask_record.py b/src/morphobricks_preprocessing/mask_record.py index b10b6370..a50d44f7 100644 --- a/src/morphobricks_preprocessing/mask_record.py +++ b/src/morphobricks_preprocessing/mask_record.py @@ -1,12 +1,13 @@ """The masks as the run manifest records them (manifest version 2, contract §9, §15). Every mask entry adds to what :meth:`.masks.MaskInventory.to_manifest` gives its -fingerprint (``size``, ``mtime_ns``), so a brick can tell a mask changed since the run, and +fingerprint (``size``, ``mtime_ns``, and ``inventory_sha256`` for an OME-Zarr label image, +issue #86), so a brick can tell a mask changed since the run, and how it is read: ``roles`` (one per axis of ``axes``), ``axes_from`` (``"header"``: the mask's metadata names them; ``"stated"``: the user stated them for its mask type) and ``shared`` (a 2-D mask stated as shared by every plane of its image; never without a statement). A label image of an OME-Zarr image has its own fingerprint, left out of its -image's (:func:`.manifest.source_fingerprint`). +image's (:func:`.inventory.source_signature`). """ from __future__ import annotations @@ -52,8 +53,7 @@ def mask_manifest(inventory: MaskInventory, experiment_root: str | os.PathLike) fingerprint = Fingerprint.of(_mask_path(mask, inventory.folder, Path(experiment_root))) entry.update( axes=mask.header_axes or mask.axes, - size=fingerprint.size, - mtime_ns=fingerprint.mtime_ns, + **fingerprint.to_data(), roles=mask.axes, axes_from=mask.axes_from or "header", shared=mask.shared, diff --git a/src/morphobricks_preprocessing/notebook_run.py b/src/morphobricks_preprocessing/notebook_run.py index 4cb834f1..4074c2e3 100644 --- a/src/morphobricks_preprocessing/notebook_run.py +++ b/src/morphobricks_preprocessing/notebook_run.py @@ -400,6 +400,7 @@ def _show_run_tiles(self, result: RunResult) -> None: tiles = [ ("🧩", f"{result.plane_count:,}", "planes in the table"), ("📖", f"{len(result.read):,}", "images read"), + ("⏯️", f"{len(result.resumed):,}", "images resumed from an interrupted run"), ("♻️", f"{kept:,}", "images kept from the last run"), ("🆕", f"{len(result.new):,}", "new images"), ("✏️", f"{len(result.changed):,}", "changed images"), diff --git a/src/morphobricks_preprocessing/report.py b/src/morphobricks_preprocessing/report.py index 19ea4db1..ff15c2ce 100644 --- a/src/morphobricks_preprocessing/report.py +++ b/src/morphobricks_preprocessing/report.py @@ -284,6 +284,7 @@ def run_summary(result: RunResult, *, design_template: bool = False) -> str: lines.append(f"All rows were computed because {result.rebuild_reason}.") for label, files in ( ("Read", result.read), + ("Resumed (rows staged by an interrupted run)", result.resumed), ("New since the last run", result.new), ("Changed since the last run", result.changed), ("Unchanged (rows kept)", () if result.rebuild_reason else result.unchanged), diff --git a/src/morphobricks_preprocessing/run/entries.py b/src/morphobricks_preprocessing/run/entries.py index 1f93d2a8..e163b8f6 100644 --- a/src/morphobricks_preprocessing/run/entries.py +++ b/src/morphobricks_preprocessing/run/entries.py @@ -11,14 +11,15 @@ import logging from collections.abc import Mapping, Sequence -from dataclasses import dataclass, replace +from dataclasses import astuple, dataclass, replace from pathlib import Path from morphobricks_normalization.records import PlaneRecord from ..axes import AxisReview from ..fingerprint import RowKey -from ..manifest import FileEntry, RunManifest, RunSettings, file_entry_for +from ..inventory import source_signature +from ..manifest import FileEntry, RunManifest, RunSettings, file_entry_with from .preflight import Preflight logger = logging.getLogger(__name__) @@ -50,12 +51,14 @@ def file_entries( entries: dict[str, FileEntry] = {} for rel, file_mapping in sorted(review.file_mappings.items()): header = headers[rel] - entry = file_entry_for( - root, + # The fingerprint taken with the header (issue #90), else as it is now. + bound = pre.header_fingerprints.get(rel) + entry = file_entry_with( + source_signature(root / rel) if bound is None else astuple(bound), rel, labels[rel], file_mapping, - channel_names=header.channel_names, + channel_names=tuple(header.channel_names), significant_bits=header.significant_bits, read_in_blocks=rel in in_blocks, channel_names_trimmed=header.channel_names_trimmed, diff --git a/src/morphobricks_preprocessing/run/generation.py b/src/morphobricks_preprocessing/run/generation.py new file mode 100644 index 00000000..fed4486a --- /dev/null +++ b/src/morphobricks_preprocessing/run/generation.py @@ -0,0 +1,298 @@ +"""Publish a run's outputs as one generation, made current in one atomic step (contract §9). + +A run's published outputs (``plane_histograms.csv``, ``experiment_design.csv``, +``run_manifest.json`` and the report files) are one **generation**. Publication is +crash-consistent (owner decision of 2 November 2026, PP-12): + +1. The whole new generation is written into the publication folder ``.publish`` in the + output folder, each file flushed to disk, and then the **publication record** + ``.publish/publication.json`` naming every file with its SHA-256 (and the files the + generation removes, such as ``mask_relationships.json``). +2. **Commit, the one atomic step:** the new ``run_manifest.json`` replaces the old one + (:func:`os.replace`). Before it nothing in the output folder has changed: a run + killed then leaves the earlier generation exactly as it was, and the next run discards + the publication folder. After it the new generation is the current one. +3. The other files are moved into place (:func:`os.replace` each), the removed files + removed, and the publication folder deleted. + +A run killed during step 3 leaves the new manifest with some files still in ``.publish``. +The next run, ``check``, ``masks``, ``design`` or ``calibrate`` (whatever takes the output +folder's lock) finishes the moves first (:func:`finish_publication`). Readers that go +through the manifest (``RunInputs``) never see a mix: they read each file the manifest +names by its SHA-256, from the output folder or, while a publication is unfinished, from +``.publish`` (:func:`published_path`). A plain reader of ``plane_histograms.csv`` alone +always reads one whole table of one generation (each move is one rename), the old one +until its move; one that reads several files without the manifest can pair files of two +generations during step 3 or after a kill in it, until the next run finishes it. +""" + +from __future__ import annotations + +import json +import logging +import os +import shutil +from collections.abc import Callable, Iterable, Iterator +from contextlib import contextmanager +from pathlib import Path +from typing import TypeVar + +from morphobricks_normalization.csv_io import OUTPUT_FILENAME + +from .._io import give_output_mode, write_text_atomic +from ..design import DESIGN_FILENAME +from ..discovery_report import REPORT_FILENAME, REPORT_TEXT_FILENAME +from ..manifest import MANIFEST_FILENAME, file_sha256 + +logger = logging.getLogger(__name__) + +T = TypeVar("T") + +PUBLISH_FOLDER = ".publish" +"""The publication folder in the output folder, holding a generation being published.""" +PUBLICATION_RECORD = "publication.json" +"""The record of a generation being published, written last into :data:`PUBLISH_FOLDER`.""" +PUBLICATION_VERSION = "1" +MOVED_IN_ORDER = (OUTPUT_FILENAME, DESIGN_FILENAME, REPORT_TEXT_FILENAME, REPORT_FILENAME) +"""The files moved into place after the commit, in this order (the manifest is the commit).""" + + +class PublicationIncompleteError(RuntimeError): + """Raised when a committed generation's files cannot all be moved into place. + + The new generation is the current one (its manifest is in place) and readers that go + through the manifest read it; the next run that takes the output folder's lock finishes + the moves. + """ + + +@contextmanager +def staged_generation(output: Path, removes: Iterable[str] = ()) -> Iterator[Path]: + """Give a fresh publication folder to write a generation into, then publish it. + + Parameters + ---------- + output + The output folder (its lock held). + removes + Output files the new generation removes (for example ``mask_relationships.json``). + + Yields + ------ + Path + The publication folder: write the manifest there and any of + :data:`MOVED_IN_ORDER`. When the block ends without an exception, the generation + is published (:func:`publish_generation`); on an exception the folder is removed + and nothing in the output folder has changed. + """ + finish_publication(output) + staged = output / PUBLISH_FOLDER + staged.mkdir() + try: + yield staged + _commit(output, staged, tuple(removes)) + except BaseException: + # Not committed (or interrupted right after the commit): discarded, or finished. + try: + finish_publication(output) + except Exception: + logger.warning("Cannot clean up the publication in %s", staged, exc_info=True) + raise + _finish_committed(output, staged) + + +def _commit(output: Path, staged: Path, removes: tuple[str, ...]) -> None: + """Write the publication record, then replace the manifest: the one atomic step. + + Nothing in the output folder changes before the replacement, which either happens + whole or not at all. + """ + names = [name for name in (*MOVED_IN_ORDER, MANIFEST_FILENAME) if (staged / name).exists()] + if MANIFEST_FILENAME not in names: + raise ValueError("a generation needs its manifest") + for name in names: + _flush(staged / name) + record = { + "publication_version": PUBLICATION_VERSION, + "files": {name: file_sha256(staged / name) for name in names}, + "removes": sorted(removes), + } + write_text_atomic(staged / PUBLICATION_RECORD, json.dumps(record, indent=2) + "\n") + _flush(staged / PUBLICATION_RECORD) + _flush_folder(staged) + give_output_mode(staged / MANIFEST_FILENAME, output / MANIFEST_FILENAME) + os.replace(staged / MANIFEST_FILENAME, output / MANIFEST_FILENAME) + + +def finish_publication(output: Path) -> str | None: + """Finish or discard a publication an earlier run left in the output folder. + + Called by everything that takes the output folder's lock, before it reads or writes + the outputs. A publication whose manifest is in place (committed) is finished: its + remaining files are moved into place and the files it removes removed. Any other + publication folder (not committed: the run was killed before its manifest replaced + the earlier one) is deleted; the earlier generation is then intact. + + Parameters + ---------- + output + The output folder (its lock held). + + Returns + ------- + str or None + ``"finished"``, ``"discarded"``, or None when there was no publication folder. + + Raises + ------ + PublicationIncompleteError + If a committed generation's files cannot be moved into place. + """ + staged = output / PUBLISH_FOLDER + if not staged.exists(): + return None + record = _record(staged) + manifest = output / MANIFEST_FILENAME + committed = ( + record is not None + and manifest.is_file() + and file_sha256(manifest) == record["files"].get(MANIFEST_FILENAME) + ) + if not committed: + logger.info("Discarding the unfinished publication in %s", staged) + shutil.rmtree(staged) + return "discarded" + logger.info("Finishing the publication an earlier run left in %s", staged) + _finish_committed(output, staged) + return "finished" + + +def _finish_committed(output: Path, staged: Path) -> None: + """Move a committed generation's files into place, remove what it removes, clean up.""" + record = _record(staged) or {"removes": []} + try: + _flush_folder(output) + for name in MOVED_IN_ORDER: + if (staged / name).exists(): + give_output_mode(staged / name, output / name) + os.replace(staged / name, output / name) + for name in record["removes"]: + (output / name).unlink(missing_ok=True) + except OSError as exc: + raise PublicationIncompleteError( + f"The new results are published ({MANIFEST_FILENAME} names them), but some " + f"files could not be moved into place from {staged} ({exc}). Close programs " + "using the output files and run again: the next run finishes the move first. " + "Later bricks already read the new results." + ) from exc + try: + shutil.rmtree(staged) + except OSError: + # Every file is in place: a leftover folder is removed by the next run. + logger.warning("Cannot remove the publication folder %s", staged, exc_info=True) + + +def published_path(output: Path, name: str, sha256: str) -> Path: + """Return where the file of the current generation named ``name`` is (for readers). + + Parameters + ---------- + output + The output folder. + name + The output file, for example ``plane_histograms.csv``. + sha256 + Its SHA-256 as the current manifest records it. + + Returns + ------- + Path + The file in the output folder when its SHA-256 is ``sha256``; else the copy an + unfinished publication holds in :data:`PUBLISH_FOLDER` when its SHA-256 is; else + the file in the output folder (the caller's check then refuses it). + """ + path = output / name + if _has_sha(path, sha256): + return path + pending = output / PUBLISH_FOLDER / name + return pending if _has_sha(pending, sha256) else path + + +def read_stable(output: Path, read: Callable[[], T], attempts: int = 3) -> T | None: + """Call ``read`` until no publication happened while it read (for readers). + + A publication changes the manifest before it moves any other file into place, so a + reader that finds the same manifest before and after reading the files it names has + read one generation, whatever a run did meanwhile. + + Parameters + ---------- + output + The output folder. + read + Reads the files the manifest names (through :func:`published_path`). A + ``FileNotFoundError`` (a file moved into place meanwhile) counts as a try. + attempts + How many times to try. + + Returns + ------- + object or None + What ``read`` returned, or None when every try overlapped a publication. + """ + for _ in range(attempts): + before = _manifest_bytes(output) + try: + value = read() + except FileNotFoundError: + continue + if _manifest_bytes(output) == before: + return value + return None + + +def _manifest_bytes(output: Path) -> bytes | None: + """Return the manifest's bytes, or None when there is none.""" + try: + return (output / MANIFEST_FILENAME).read_bytes() + except FileNotFoundError: + return None + + +def _has_sha(path: Path, sha256: str) -> bool: + """Return True when ``path`` is a file whose SHA-256 is ``sha256``.""" + try: + return path.is_file() and file_sha256(path) == sha256 + except FileNotFoundError: # moved away meanwhile + return False + + +def _record(staged: Path) -> dict | None: + """Read a publication record, or None when it is absent or cannot be read.""" + try: + data = json.loads((staged / PUBLICATION_RECORD).read_text(encoding="utf-8")) + except (OSError, ValueError): + return None + if not isinstance(data, dict) or data.get("publication_version") != PUBLICATION_VERSION: + return None + files, removes = data.get("files"), data.get("removes") + if not isinstance(files, dict) or not isinstance(removes, list): + return None + return data + + +def _flush(path: Path) -> None: + """Flush a written file to the disk, so a power cut cannot commit a partial file.""" + with open(path, "rb+") as stream: + os.fsync(stream.fileno()) + + +def _flush_folder(folder: Path) -> None: + """Flush a folder's entries (POSIX only; Windows cannot open a folder).""" + if os.name != "posix": + return + handle = os.open(folder, os.O_RDONLY) + try: + os.fsync(handle) + finally: + os.close(handle) diff --git a/src/morphobricks_preprocessing/run/lock.py b/src/morphobricks_preprocessing/run/lock.py index 74437e85..a5cd4db8 100644 --- a/src/morphobricks_preprocessing/run/lock.py +++ b/src/morphobricks_preprocessing/run/lock.py @@ -19,6 +19,7 @@ from pathlib import Path from ..validation import ValidationIssue, error +from .generation import finish_publication LOCK_FILENAME = ".morphobricks-preprocessing.lock" @@ -92,7 +93,10 @@ def output_lock(output_folder: str | os.PathLike) -> Iterator[LockHolder]: """Hold the output folder's lock for the duration of the block. The folder must exist. The lock file is created with ``O_CREAT | O_EXCL``, so two - runs can never both hold it, and removed when the block ends, also on an error. + runs can never both hold it, and removed when the block ends, also on an error. Once + it is held, a publication an earlier run left unfinished is finished, or discarded + when it was never committed (:func:`.generation.finish_publication`), so whatever + holds the lock works on one whole generation of the outputs. Parameters ---------- @@ -108,6 +112,8 @@ def output_lock(output_folder: str | os.PathLike) -> Iterator[LockHolder]: ------ OutputLockedError When another run holds the lock. + PublicationIncompleteError + When an unfinished publication's files cannot be moved into place. """ path = Path(output_folder) / LOCK_FILENAME holder = LockHolder( @@ -124,6 +130,7 @@ def output_lock(output_folder: str | os.PathLike) -> Iterator[LockHolder]: path.unlink(missing_ok=True) # just created by this run: a lock it could not write raise try: + finish_publication(Path(output_folder)) yield holder finally: if read_lock(output_folder) == holder: diff --git a/src/morphobricks_preprocessing/run/pipeline.py b/src/morphobricks_preprocessing/run/pipeline.py index 352aba8d..46a7043f 100644 --- a/src/morphobricks_preprocessing/run/pipeline.py +++ b/src/morphobricks_preprocessing/run/pipeline.py @@ -53,10 +53,11 @@ from ..planes import DEFAULT_MEMORY_LIMIT from ..validation import ValidationIssue, has_errors, warning from .design_file import design_for_run +from .generation import staged_generation from .lock import OutputLockedError, lock_issue, output_lock from .modes import FULL, RESTART, check_mode from .preflight import Preflight, fit_calibration, preflight -from .publish import _staged_outputs, write_report +from .publish import write_report from .recorded import ( GivenInputs, ResolvedInputs, @@ -65,8 +66,10 @@ usable, ) from .result import RunResult +from .resume import changed_issue, changed_since from .reuse import read_previous, recorded_experiment_root from .rows import _pixel_issues, _range_issues, _row_order +from .staging import clear_staging from .steps import Checked, Rows, compute_rows, counts, header_checks, outcome_of, unreadable_issue logger = logging.getLogger(__name__) @@ -215,12 +218,15 @@ def _locked_run(given: GivenInputs, output: Path, options: _Options) -> RunResul found = frozenset(path.name for path in output.iterdir()) previous = read_previous(output) inputs = resolve_inputs(given, previous[0]) + # Each header bound to its image's fingerprint (issue #90): the rows must describe + # the file whose header was checked. pre = preflight( inputs.experiment_root, output, exclude=inputs.exclude, masks_folder=inputs.masks_folder, mask_axes=inputs.mask_axes or None, + bind_headers=True, ) design, design_problems, design_changed = design_for_run(options.design, output, previous[0]) checked = header_checks(pre, inputs, design, options.memory_limit) @@ -249,8 +255,11 @@ def _locked_run(given: GivenInputs, output: Path, options: _Options) -> RunResul histogram=inputs.histogram_settings, ) rows = compute_rows(pre, checked, settings, output, previous, options.mode, options.progress) + if rows.changed: + checked.issues.append(changed_issue(list(rows.changed))) if rows.failed: checked.issues.append(unreadable_issue(rows.failed)) + if rows.changed or rows.failed: return _stop(run, base, rows) checked.issues.extend(_pixel_issues(rows.records)) rows.narrow.sort(key=lambda entry: (entry.plane_id, entry.channel)) @@ -297,48 +306,36 @@ def _stop(run: _Run, base: RunOutcome, rows: Rows | None) -> RunResult: def _publish(run: _Run, base: RunOutcome, rows: Rows, settings: RunSettings) -> RunResult: - """Publish the table, design, manifest and report together, and return the result.""" + """Publish the table, design, manifest and report as one generation; return the result. + + Every image is fingerprinted again just before the commit (issue #90): an image changed + since the run checked it stops the run, and nothing is published. + """ output, pre, design = run.output, run.pre, run.checked.design records = sorted(rows.records, key=_row_order) # From the rows in memory: no image is read again. intensity = check_intensities(records, run.inputs.intensity_settings) - per_file = Counter(record.source_relative_path for record in records) - root = pre.discovery.root - masks = None if pre.masks is None else mask_manifest(pre.masks, root) + masks = None if pre.masks is None else mask_manifest(pre.masks, pre.discovery.root) outcome = replace(outcome_of(rows, base), replaced=_replaced(run, masks)) - with _staged_outputs(output, with_design=design is not None, with_report=True) as staged: - csv_path = write_plane_histograms(records, staged / OUTPUT_FILENAME) - design_sha256 = None - if design is not None: - design_sha256 = file_sha256(write_experiment_design(design, staged / DESIGN_FILENAME)) - manifest = RunManifest( - experiment_root=root.as_posix(), - dtype=pre.metadata.dtype.name, - settings=settings, - mapping=run.checked.review.mapping, - files=tuple( - replace(entry, plane_count=per_file[rel]) - for rel, entry in sorted(rows.entries.items()) - ), - csv_sha256=file_sha256(csv_path), - versions=package_versions(), - written_at=datetime.now(UTC).isoformat(timespec="seconds"), - experiment_design_sha256=design_sha256, - masks=masks, - image_type=run.inputs.image_type, - channels_from_rgb_samples=run.checked.type_review.channels_from_rgb_samples, - intensity_checks=intensity.settings, - narrow_ranges=tuple(rows.narrow), - calibration_sha256=pre.calibration.sha256, - conversion_record_sha256=None if pre.conversion is None else pre.conversion.sha256, - ) - manifest_sha256 = file_sha256(write_manifest(manifest, staged / MANIFEST_FILENAME)) - report = _report(run, outcome, intensity) - write_report(replace(report, status=WRITTEN, manifest_sha256=manifest_sha256), staged) - if MASK_REPORT_FILENAME in outcome.replaced: - (output / MASK_REPORT_FILENAME).unlink(missing_ok=True) - kept = len(records) - sum(per_file[rel] for rel in rows.read) - logger.info("Wrote %d planes (%d files read, %d kept)", len(records), len(rows.read), kept) + removes = [name for name in (MASK_REPORT_FILENAME,) if name in outcome.replaced] + try: + with staged_generation(output, removes) as staged: + sha256 = _write_generation(run, rows, settings, records, (masks, intensity), staged) + changed = changed_since(rows.entries, pre.discovery.root) + if changed: + raise _SourcesChangedError(changed) + report = replace(_report(run, outcome, intensity), status=WRITTEN) + write_report(replace(report, manifest_sha256=sha256), staged) + except _SourcesChangedError as exc: + run.checked.issues.append(changed_issue(exc.paths)) + return _stop(run, base, rows) + clear_staging(output) + logger.info( + "Wrote %d planes (%d files read, %d resumed)", + len(records), + len(rows.read), + len(rows.resumed), + ) return RunResult( output, True, @@ -356,6 +353,57 @@ def _publish(run: _Run, base: RunOutcome, rows: Rows, settings: RunSettings) -> ) +class _SourcesChangedError(Exception): + """Raised inside the publication when images changed since the run checked them.""" + + def __init__(self, paths: list[str]) -> None: + super().__init__(", ".join(paths)) + self.paths = paths + + +def _write_generation( + run: _Run, + rows: Rows, + settings: RunSettings, + records: list, + checks: tuple[dict | None, IntensityReport], + staged: Path, +) -> str: + """Write the table, design and manifest into the publication folder. + + ``checks`` are the masks entry and the intensity checks. Returns the manifest's + SHA-256 (the report, which names it, is written once the images are checked again). + """ + pre, design = run.pre, run.checked.design + masks, intensity = checks + per_file = Counter(record.source_relative_path for record in records) + csv_path = write_plane_histograms(records, staged / OUTPUT_FILENAME) + design_sha256 = None + if design is not None: + design_sha256 = file_sha256(write_experiment_design(design, staged / DESIGN_FILENAME)) + manifest = RunManifest( + experiment_root=pre.discovery.root.as_posix(), + dtype=pre.metadata.dtype.name, + settings=settings, + mapping=run.checked.review.mapping, + files=tuple( + replace(entry, plane_count=per_file[rel]) for rel, entry in sorted(rows.entries.items()) + ), + csv_sha256=file_sha256(csv_path), + versions=package_versions(), + written_at=datetime.now(UTC).isoformat(timespec="seconds"), + experiment_design_sha256=design_sha256, + masks=masks, + image_type=run.inputs.image_type, + channels_from_rgb_samples=run.checked.type_review.channels_from_rgb_samples, + intensity_checks=intensity.settings, + narrow_ranges=tuple(rows.narrow), + calibration_sha256=pre.calibration.sha256, + conversion_record_sha256=None if pre.conversion is None else pre.conversion.sha256, + ) + return file_sha256(write_manifest(manifest, staged / MANIFEST_FILENAME)) + + def _replaced(run: _Run, masks: dict | None) -> dict[str, str]: """Return the earlier output files the run replaces or removes, and why. diff --git a/src/morphobricks_preprocessing/run/publish.py b/src/morphobricks_preprocessing/run/publish.py index 6475b2ca..189ae615 100644 --- a/src/morphobricks_preprocessing/run/publish.py +++ b/src/morphobricks_preprocessing/run/publish.py @@ -1,42 +1,18 @@ -"""Publish a run's outputs together, restoring the earlier ones on failure (contract §9). +"""Write a run's report; the run's outputs are published as one generation (contract §9). -The table, the design, the manifest and the report are published together, the report -last, only when the run succeeds; before that nothing is erased (design v0.4 §1). A run -that stops writes only its report (:func:`write_report`). Moved from ``run.py`` (PP-3). +The table, the design, the manifest and the report are published together, only when the +run succeeds, as one generation made current in one atomic step (:mod:`.generation`); +before that nothing is erased (design v0.4 §1). A run that stops writes only its report +(:func:`write_report`). Moved from ``run.py`` (PP-3). """ from __future__ import annotations import json -import logging -import os -import shutil -import tempfile -from collections.abc import Iterator -from contextlib import contextmanager from pathlib import Path -from morphobricks_normalization.csv_io import ( - OUTPUT_FILENAME, -) - -from .._io import give_output_mode, write_text_atomic -from ..design import ( - DESIGN_FILENAME, -) +from .._io import write_text_atomic from ..discovery_report import REPORT_FILENAME, REPORT_TEXT_FILENAME, DiscoveryReport -from ..manifest import ( - MANIFEST_FILENAME, -) - -logger = logging.getLogger(__name__) - -STAGING_PREFIX = ".run-" -"""The name prefix of a run's staging folders in the output folder (removed after the run).""" - - -class _OutputRollbackError(RuntimeError): - """Raised when an output update fails and its earlier files cannot be restored.""" def write_report(report: DiscoveryReport, folder: Path) -> Path: @@ -47,7 +23,7 @@ def write_report(report: DiscoveryReport, folder: Path) -> Path: report The report. folder - The output folder, or the run's staging folder. + The output folder, or the publication folder of the run's generation. Returns ------- @@ -57,74 +33,3 @@ def write_report(report: DiscoveryReport, folder: Path) -> Path: write_text_atomic(folder / REPORT_TEXT_FILENAME, report.text()) text = json.dumps(report.to_dict(), indent=2, ensure_ascii=False) + "\n" return write_text_atomic(folder / REPORT_FILENAME, text) - - -@contextmanager -def _staged_outputs( - output: Path, *, with_design: bool = False, with_report: bool = False -) -> Iterator[Path]: - """Prepare all outputs before publishing, and retain backups if recovery fails. - - The outputs are written to a temporary folder inside ``output``; ``with_design`` - publishes ``experiment_design.csv`` too, and ``with_report`` the report files. - """ - staged = Path(tempfile.mkdtemp(prefix=STAGING_PREFIX, dir=output)) - keep_backups = False - try: - yield staged - _publish_outputs(output, staged, with_design, with_report) - except _OutputRollbackError: - keep_backups = True - raise - finally: - if not keep_backups: - try: - shutil.rmtree(staged) - except OSError: - # A leftover temporary directory does not invalidate completed output. - logger.warning("Cannot remove temporary output folder %s", staged, exc_info=True) - - -def _publish_outputs( - output: Path, staged: Path, with_design: bool = False, with_report: bool = False -) -> None: - """Replace the outputs, restoring their previous state on a publication failure. - - The manifest is published after the table and design, so a manifest never names a - design that is not in place; the report, which names the manifest, comes last. - """ - report = (REPORT_TEXT_FILENAME, REPORT_FILENAME) if with_report else () - filenames = ( - OUTPUT_FILENAME, - *((DESIGN_FILENAME,) if with_design else ()), - MANIFEST_FILENAME, - *report, - ) - backup = staged / "previous" - backup.mkdir() - # Copy before replacing either file, so even a backup failure leaves the - # earlier output intact. Keep backups on the same filesystem for atomic restores. - for name in filenames: - previous = output / name - if previous.exists() or previous.is_symlink(): - shutil.copy2(previous, backup / name, follow_symlinks=False) - published: list[str] = [] - try: - for name in filenames: - give_output_mode(staged / name, output / name) - os.replace(staged / name, output / name) - published.append(name) - except BaseException: - try: - for name in reversed(published): - if (backup / name).exists() or (backup / name).is_symlink(): - os.replace(backup / name, output / name) - else: - (output / name).unlink() - except BaseException as exc: - raise _OutputRollbackError( - "The output update failed and the earlier files could not be fully restored. " - f"Backup files have been kept in {backup}. Close programs using the output " - "files and restore the backups before running again." - ) from exc - raise diff --git a/src/morphobricks_preprocessing/run/result.py b/src/morphobricks_preprocessing/run/result.py index c106ce59..d1c1745c 100644 --- a/src/morphobricks_preprocessing/run/result.py +++ b/src/morphobricks_preprocessing/run/result.py @@ -41,6 +41,9 @@ class RunResult: Without a usable earlier manifest, every file is new. read Files whose pixels were read in this run. + resumed + Files whose rows an interrupted run had staged and this run reused, without + reading their pixels (design v0.4 §1). excluded Images the user excluded. rebuild_reason @@ -76,6 +79,7 @@ class RunResult: unchanged: tuple[str, ...] = () removed: tuple[str, ...] = () read: tuple[str, ...] = () + resumed: tuple[str, ...] = () excluded: tuple[str, ...] = () rebuild_reason: str | None = None plane_count: int = 0 diff --git a/src/morphobricks_preprocessing/run/resume.py b/src/morphobricks_preprocessing/run/resume.py new file mode 100644 index 00000000..f7c7b0d9 --- /dev/null +++ b/src/morphobricks_preprocessing/run/resume.py @@ -0,0 +1,255 @@ +"""Read each file that needs new rows, or resume its staged rows (design v0.4 §1, PP-12). + +Every file a run must compute rows for is either **resumed** (its rows staged by an +interrupted run, with the same row key: :mod:`.staging`) or **read**, and its rows staged +as soon as it is read. Whether a file stayed the file the run checked is decided by its +fingerprint (issue #90): its manifest entry records the fingerprint taken when its header +was read (between two fingerprints that must agree), and the file must still have it just +after its pixels are read. A file that changed is never staged or published: it is listed, and the +run stops with ``source_changed_during_run`` (:func:`changed_issue`). +""" + +from __future__ import annotations + +import logging +from collections.abc import Callable, Mapping +from dataclasses import dataclass, field +from pathlib import Path + +from morphobricks_normalization.records import PlaneRecord + +from ..axes import AxisReview +from ..fingerprint import Fingerprint, RowKey, current_fingerprint +from ..manifest import FileEntry, NarrowRange, RunSettings +from ..planes import PlaneReadError +from ..validation import ValidationIssue, error, plural +from .preflight import Preflight +from .rows import _read_file +from .staging import discard_staged, load_staged, stage_file, staging_key + +logger = logging.getLogger(__name__) + +SOURCE_CHANGED = "source_changed_during_run" + + +@dataclass +class NewRows: + """The rows of the files a run computed or resumed. + + Attributes + ---------- + records, narrow + Their rows and narrow-range checks. + read + The files whose pixels were read. + resumed + The files whose staged rows were reused. + failed + The files whose pixels could not be read. + changed + The files that changed while the run checked or read them. + discarded + ``{file: why}`` for the staged rows not reused. + """ + + records: list[PlaneRecord] = field(default_factory=list) + narrow: list[NarrowRange] = field(default_factory=list) + read: list[str] = field(default_factory=list) + resumed: list[str] = field(default_factory=list) + failed: list[str] = field(default_factory=list) + changed: list[str] = field(default_factory=list) + discarded: dict[str, str] = field(default_factory=dict) + + +@dataclass(frozen=True) +class FileJob: + """What a run needs to compute the rows of its files. + + Attributes + ---------- + pre, review + The run's preflight and axis review. + settings + The run's settings. + entries + Every analysed file's manifest entry, as it is now. + output + The output folder (holding the staging folder). + full + A ``full`` run: no staged row is reused. + """ + + pre: Preflight + review: AxisReview + settings: RunSettings + entries: Mapping[str, FileEntry] + output: Path + full: bool + + +def changed_during_check(job: FileJob) -> list[str]: + """Return the files whose entry's fingerprint is not the one taken with their header. + + Parameters + ---------- + job + The run's files. + + Returns + ------- + list of str + The files whose header is not known to describe them as fingerprinted (they + changed, or went missing, while the run checked them), sorted. + """ + bound = job.pre.header_fingerprints + return sorted( + rel + for rel, entry in job.entries.items() + if bound and bound.get(rel) != Fingerprint.recorded(entry) + ) + + +def new_rows( + job: FileJob, + to_read: list[str], + keep: set[str], + progress: Callable[[int, int, str], None] | None, +) -> NewRows: + """Resume or read every file of ``to_read``, staging the rows of each file read. + + Parameters + ---------- + job + The run's files. + to_read + The files that need new rows, sorted. + keep + The files whose rows the earlier table gives: staged rows of these are discarded. + progress + As :func:`.pipeline.run_experiment`. + + Returns + ------- + NewRows + """ + found = NewRows() + staged, found.discarded = load_staged(job.output) + for rel in sorted(set(staged) - set(to_read)): + found.discarded[rel] = ( + "the table already holds its rows" if rel in keep else "the image is not analysed" + ) + discard_staged(job.output, rel) + dtype = job.pre.metadata.dtype.name + root = job.pre.discovery.root + for done, rel in enumerate(to_read): + if progress is not None: + progress(done, len(to_read), rel) + key = staging_key(RowKey.of(job.entries[rel], job.settings, root.as_posix()), dtype) + earlier = staged.get(rel) + if earlier is not None: + now = current_fingerprint(root / rel) + unchanged = now == Fingerprint.recorded(job.entries[rel]) + if not job.full and earlier.key == key and unchanged: + found.records.extend(earlier.records) + found.narrow.extend(earlier.narrow) + found.resumed.append(rel) + continue + found.discarded[rel] = _why_discarded(earlier.key, key, job.full, unchanged) + discard_staged(job.output, rel) + _read_one(job, rel, key, found) + if progress is not None: + progress(len(to_read), len(to_read), "") + return found + + +def _read_one(job: FileJob, rel: str, key: dict, found: NewRows) -> None: + """Read one file between two fingerprints, and stage its rows when it did not change.""" + root = job.pre.discovery.root + # The entry's fingerprint is the one taken with the header: a file changed since then + # (before or during the read) has another one after the read. + expected = Fingerprint.recorded(job.entries[rel]) + labels = job.entries[rel].labels + try: + rows, ranges = _read_file( + root / rel, + rel, + labels, + job.review.file_mappings[rel], + job.settings, + job.entries[rel].position_id, + ) + except PlaneReadError: + logger.warning("Cannot read the planes of %s", rel, exc_info=True) + found.failed.append(rel) + return + if current_fingerprint(root / rel) != expected: + found.changed.append(rel) + return + stage_file(job.output, key, rows, ranges) + found.records.extend(rows) + found.narrow.extend(ranges) + found.read.append(rel) + + +def changed_since(entries: Mapping[str, FileEntry], root: Path) -> list[str]: + """Return the images whose fingerprint now is not their entry's (gone ones included). + + Parameters + ---------- + entries + The run's manifest entries, as fingerprinted when the run checked the images. + root + The experiment folder. + + Returns + ------- + list of str + The changed or missing images, sorted. + """ + return sorted( + rel + for rel, entry in entries.items() + if current_fingerprint(root / rel) != Fingerprint.recorded(entry) + ) + + +def _why_discarded(staged: dict, now: dict, full: bool, unchanged: bool) -> str: + """Say why a file's staged rows are not reused (``unchanged``: its fingerprint now).""" + if full: + return "a full run reuses no staged row" + if ( + not unchanged + or staged.get("row_key", {}).get("fingerprint") != now["row_key"]["fingerprint"] + ): + return "the image changed since its rows were staged" + if {k: staged.get(k) for k in ("versions", "schema_version")} != { + k: now[k] for k in ("versions", "schema_version") + }: + return "its rows were staged by another version" + if staged.get("dtype") != now["dtype"]: + return "the experiment's pixel type changed since its rows were staged" + return "its axis roles, labels, folder or row settings changed since its rows were staged" + + +def changed_issue(paths: list[str]) -> ValidationIssue: + """Return the error naming the images that changed while the run checked or read them. + + Parameters + ---------- + paths + The images, sorted. + + Returns + ------- + ValidationIssue + ``source_changed_during_run``. + """ + return error( + SOURCE_CHANGED, + f"{plural(len(paths), 'image')} changed or went missing while the run was checking " + "or reading them (their size, time or files are no longer those the run started " + "with), so their rows would not describe them. Nothing was published.", + paths, + "Wait until nothing is writing to these images (an acquisition or a copy still " + "running), then run again: the images already read resume from their staged rows.", + ) diff --git a/src/morphobricks_preprocessing/run/staging.py b/src/morphobricks_preprocessing/run/staging.py new file mode 100644 index 00000000..8e8f7587 --- /dev/null +++ b/src/morphobricks_preprocessing/run/staging.py @@ -0,0 +1,243 @@ +"""Per-file staging of a run's rows, so an interrupted run resumes (design v0.4 §1). + +As soon as a run has read one file, its rows are staged in the output folder's staging +folder ``.staging``: the rows as a small ``plane_histograms.csv`` written with the +normalization brick's writer (the same writer and reader as the table, so a resumed row is +the row the run would compute), then a JSON record naming the file, its row key +(:class:`.fingerprint.RowKey`: fingerprint, labels, axes, roles, ``axes_from``, read path, +bins and histogram settings), the pixel type, the table's schema version, the versions of +the two packages that compute rows, the planes whose range check failed, and the SHA-256 of +the rows file. The record is written last, so a staged entry without one is incomplete and +never used. + +The next run (after a crash, a kill or a lost Colab session) reuses a staged file's rows +only when everything its record names is what that run would read now; anything else is +discarded and reported, never reused. The staging folder is removed once a run publishes; +a run that stops keeps it, so the next one resumes. Publication itself stays atomic +(:mod:`.generation`), and a ``full`` run reuses no staged row. +""" + +from __future__ import annotations + +import hashlib +import json +import logging +import shutil +from dataclasses import asdict, dataclass +from functools import lru_cache +from pathlib import Path + +from morphobricks_normalization.csv_io import ( + SCHEMA_VERSION, + CsvFormatError, + read_plane_histograms, + write_plane_histograms, +) +from morphobricks_normalization.records import PlaneRecord + +from .._io import write_text_atomic +from ..fingerprint import RowKey +from ..manifest import NarrowRange, file_sha256, package_versions + +logger = logging.getLogger(__name__) + +STAGING_FOLDER = ".staging" +"""The staging folder in the output folder (removed once a run publishes).""" +STAGING_VERSION = "1" +_ROW_PACKAGES = ("morphobricks-preprocessing", "morphobricks-normalization") +"""The packages whose versions a staged file's rows depend on.""" + + +@dataclass(frozen=True) +class StagedFile: + """One file's staged rows. + + Attributes + ---------- + relative_path + The image, relative to the experiment folder. + key + What the rows depend on (:func:`staging_key`), as JSON data. + records + Its rows, one per XYC plane. + narrow + Its channel planes whose range check found the central range too narrow. + """ + + relative_path: str + key: dict + records: tuple[PlaneRecord, ...] + narrow: tuple[NarrowRange, ...] + + +def staging_key(key: RowKey, dtype: str) -> dict: + """Return everything a file's staged rows depend on, as JSON data. + + Parameters + ---------- + key + The file's row key with the run's settings. + dtype + The experiment's pixel type. + + Returns + ------- + dict + The row key, the pixel type, the table's schema version and the versions of the + packages that compute rows, normalised through JSON so it compares with a record. + """ + data = { + "row_key": asdict(key), + "dtype": dtype, + "schema_version": SCHEMA_VERSION, + "versions": dict(_row_versions()), + } + return json.loads(json.dumps(data, sort_keys=True)) + + +def stage_file( + output: Path, + key: dict, + records: list[PlaneRecord], + narrow: list[NarrowRange], +) -> None: + """Stage one file's rows: its rows file first, then the record naming them. + + Parameters + ---------- + output + The output folder (its lock held). + key + :func:`staging_key` of the file as it was read. + records, narrow + Its rows and its narrow-range checks. + """ + folder = output / STAGING_FOLDER + folder.mkdir(exist_ok=True) + relative = records[0].source_relative_path + stem = _stem(relative) + rows = write_plane_histograms(records, folder / f"{stem}.csv") + record = { + "staging_version": STAGING_VERSION, + "relative_path": relative, + "key": key, + "rows_sha256": file_sha256(rows), + "narrow_histogram_ranges": [entry.to_dict() for entry in narrow], + } + write_text_atomic(folder / f"{stem}.json", json.dumps(record, sort_keys=True) + "\n") + + +def load_staged(output: Path) -> tuple[dict[str, StagedFile], dict[str, str]]: + """Read every staged file of an earlier, interrupted run. + + Parameters + ---------- + output + The output folder. + + Returns + ------- + tuple + ``({relative path: StagedFile}, {image: why it was discarded})`` (the record's + file name when it does not name its image). A record that + cannot be read, or whose rows file is missing or not the one it names, is + discarded (and removed); a rows file without its record (a run killed between the + two writes) is removed without a report: it was never staged. + """ + folder = output / STAGING_FOLDER + staged: dict[str, StagedFile] = {} + discarded: dict[str, str] = {} + if not folder.is_dir(): + return staged, discarded + for path in sorted(folder.glob("*.json")): + try: + found = _read_staged(path) + except (OSError, ValueError, KeyError, TypeError, CsvFormatError) as exc: + logger.warning("Cannot read the staged rows %s", path, exc_info=True) + discarded[_staged_name(path)] = f"its staged rows could not be read ({exc})" + _remove(path) + continue + staged[found.relative_path] = found + named = {_stem(rel) for rel in staged} + for rows in folder.glob("*.csv"): + if rows.stem not in named: + _remove(rows.with_suffix(".json")) + return staged, discarded + + +def discard_staged(output: Path, relative_path: str) -> None: + """Remove one file's staged rows (record first, so nothing half-staged is left). + + Parameters + ---------- + output + The output folder. + relative_path + The image. + """ + stem = output / STAGING_FOLDER / _stem(relative_path) + stem.with_suffix(".json").unlink(missing_ok=True) + stem.with_suffix(".csv").unlink(missing_ok=True) + + +def clear_staging(output: Path) -> None: + """Remove the staging folder, once a run has published. + + Parameters + ---------- + output + The output folder. + """ + folder = output / STAGING_FOLDER + if not folder.exists(): + return + try: + shutil.rmtree(folder) + except OSError: + # Its rows are in the published table; the next run discards what is left. + logger.warning("Cannot remove the staging folder %s", folder, exc_info=True) + + +@lru_cache(maxsize=1) +def _row_versions() -> tuple[tuple[str, str], ...]: + """Return the versions of the packages that compute rows (read once per process).""" + versions = package_versions() + return tuple((name, versions[name]) for name in _ROW_PACKAGES) + + +def _read_staged(path: Path) -> StagedFile: + """Read one staged record and its rows, checking the rows are the ones it names.""" + data = json.loads(path.read_text(encoding="utf-8")) + if data["staging_version"] != STAGING_VERSION: + raise ValueError(f"staging version {data['staging_version']!r}") + relative = str(data["relative_path"]) + rows = path.with_suffix(".csv") + if path.stem != _stem(relative) or file_sha256(rows) != data["rows_sha256"]: + raise ValueError("its rows file is not the one it names") + records = tuple(read_plane_histograms(rows)) + if not records or any(r.source_relative_path != relative for r in records): + raise ValueError("its rows are not the file's") + narrow = tuple(NarrowRange.from_dict(e) for e in data["narrow_histogram_ranges"]) + return StagedFile(relative, dict(data["key"]), records, narrow) + + +def _staged_name(record: Path) -> str: + """Return the image a staged record names, or the record's file name when unreadable.""" + try: + relative = json.loads(record.read_text(encoding="utf-8"))["relative_path"] + except (OSError, ValueError, KeyError, TypeError): + return record.name + if isinstance(relative, str) and record.stem == _stem(relative): + return relative + return record.name + + +def _remove(record: Path) -> None: + """Remove a staged record and its rows file.""" + record.unlink(missing_ok=True) + record.with_suffix(".csv").unlink(missing_ok=True) + + +def _stem(relative_path: str) -> str: + """Return the file name of a staged file (its path may hold folders and any letters).""" + return hashlib.sha256(relative_path.encode("utf-8")).hexdigest()[:32] diff --git a/src/morphobricks_preprocessing/run/steps.py b/src/morphobricks_preprocessing/run/steps.py index f739a658..28347ac1 100644 --- a/src/morphobricks_preprocessing/run/steps.py +++ b/src/morphobricks_preprocessing/run/steps.py @@ -8,8 +8,7 @@ from __future__ import annotations -import logging -from collections.abc import Callable +from collections.abc import Callable, Mapping from dataclasses import asdict, dataclass, field, replace from datetime import datetime from pathlib import Path @@ -25,15 +24,15 @@ from ..image_type import ImageTypeReview, image_type_issues, review_image_type from ..manifest import FileEntry, NarrowRange, RunManifest, RunSettings from ..mask_checks import mask_issues -from ..planes import PlaneReadError, check_planes +from ..planes import check_planes from ..validation import ValidationIssue, error, has_errors, plural from .entries import Comparison, compare_with_previous, file_entries +from .modes import FULL from .preflight import Preflight from .recorded import ResolvedInputs +from .resume import FileJob, changed_during_check, new_rows from .reuse import _load_previous -from .rows import _bin_count, _read_file - -logger = logging.getLogger(__name__) +from .rows import _bin_count @dataclass(frozen=True) @@ -156,13 +155,19 @@ class Rows: found The files compared with the earlier run's. read - The files read in this run. + The files whose pixels were read in this run. reason Why every row was recomputed, or None. failed Files whose pixels could not be read. entries The manifest entry of every analysed file, as it is now. + resumed + The files whose rows an interrupted run had staged, reused (design v0.4 §1). + changed + The files that changed while the run checked or read them (issue #90). + discarded + ``{file: why}`` for the staged rows not reused. """ records: list[PlaneRecord] @@ -172,6 +177,9 @@ class Rows: reason: str | None failed: tuple[str, ...] = () entries: dict[str, FileEntry] = field(default_factory=dict) + resumed: tuple[str, ...] = () + changed: tuple[str, ...] = () + discarded: Mapping[str, str] = field(default_factory=dict) def compute_rows( @@ -183,7 +191,7 @@ def compute_rows( mode: str, progress: Callable[[int, int, str], None] | None, ) -> Rows: - """Reuse the earlier rows that can be reused, and read every other file. + """Reuse the earlier rows that can be reused, resume staged rows, and read the rest. Parameters ---------- @@ -220,26 +228,27 @@ def compute_rows( records.extend(replace(r, position_id=entries[rel].position_id) for r in earlier[rel]) # Kept rows keep the range checks they were written with. narrow.extend(entry for entry in previous[0].narrow_ranges if entry.relative_path == rel) - to_read = sorted([*found.new, *found.changed, *(found.unchanged if reason else ())]) - labels = {image: leaf.labels for leaf in pre.discovery.leaves for image in leaf.images} - failed = [] - for done, rel in enumerate(to_read): - if progress is not None: - progress(done, len(to_read), rel) - file_mapping, position = review.file_mappings[rel], entries[rel].position_id - try: - rows, ranges = _read_file( - root / rel, rel, labels[rel], file_mapping, settings, position - ) - except PlaneReadError: - logger.warning("Cannot read the planes of %s", rel, exc_info=True) - failed.append(rel) - continue - records.extend(rows) - narrow.extend(ranges) - if progress is not None: - progress(len(to_read), len(to_read), "") - return Rows(records, narrow, found, tuple(to_read), reason, tuple(failed), entries) + job = FileJob(pre, review, settings, entries, output, mode == FULL) + changed = changed_during_check(job) + to_read = [ + rel + for rel in sorted([*found.new, *found.changed, *(found.unchanged if reason else ())]) + if rel not in changed + ] + kept = set(found.unchanged) if not reason else set() + new = new_rows(job, to_read, kept, progress) + return Rows( + [*records, *new.records], + [*narrow, *new.narrow], + found, + tuple(new.read), + reason, + tuple(new.failed), + entries, + resumed=tuple(new.resumed), + changed=tuple(sorted({*changed, *new.changed})), + discarded=dict(sorted(new.discarded.items())), + ) def unreadable_issue(failed: tuple[str, ...]) -> ValidationIssue: @@ -274,11 +283,23 @@ def outcome_of(rows: Rows | None, base: RunOutcome) -> RunOutcome: why.update({rel: "changed since the earlier run" for rel in found.changed}) recomputed = {rel: rows.reason or why.get(rel, "") for rel in rows.read} reused = () if rows.reason else found.unchanged - return replace(base, reused=reused, recomputed=recomputed, removed=found.removed) + return replace( + base, + reused=reused, + recomputed=recomputed, + removed=found.removed, + resumed=rows.resumed, + staged_discarded=rows.discarded, + ) def counts(rows: Rows | None) -> dict: """Return the :class:`.result.RunResult` fields that come from the rows.""" if rows is None: return {} - return {**asdict(rows.found), "rebuild_reason": rows.reason, "read": rows.read} + return { + **asdict(rows.found), + "rebuild_reason": rows.reason, + "read": rows.read, + "resumed": rows.resumed, + } diff --git a/src/morphobricks_preprocessing/run_inputs.py b/src/morphobricks_preprocessing/run_inputs.py index 814cbc9a..5d9a14c1 100644 --- a/src/morphobricks_preprocessing/run_inputs.py +++ b/src/morphobricks_preprocessing/run_inputs.py @@ -52,8 +52,15 @@ from .masks.header import _mask_header from .planes import DEFAULT_MEMORY_LIMIT, read_plane from .recorded_sizes import RecordedSize +from .run.generation import published_path, read_stable +from .run_inputs_text import ( + _ZARR_MISSING, + _changed_text, + _labels_unreadable_text, + _unrecorded_text, + _without_axes_text, +) from .unrecorded import UnrecordedFiles, unrecorded_files -from .validation import plural from .zarr_input import NgffError, ZarrUnavailableError @@ -181,24 +188,42 @@ def open( excluded (added since the run): it names them. """ output = Path(output_folder).resolve() + # One generation of the outputs (contract §9): read again if a run published + # meanwhile. + inputs = read_stable(output, lambda: cls._read(output, experiment_root, masks_root)) + if inputs is None: + raise RunInputsError( + f"Another run kept publishing new results in {output} while they were being " + "opened. Open the run again once it has finished." + ) + inputs._check_files() + return inputs + + @classmethod + def _read( + cls, + output: Path, + experiment_root: str | os.PathLike | None, + masks_root: str | os.PathLike | None, + ) -> RunInputs: + """Read the manifest and the files it names, checking their SHA-256.""" manifest = _manifest(output) - _check_sha(output / OUTPUT_FILENAME, manifest.csv_sha256, "histogram table") + table = published_path(output, OUTPUT_FILENAME, manifest.csv_sha256) + _check_sha(table, manifest.csv_sha256, "histogram table") if manifest.experiment_design_sha256 is not None: - _check_sha(output / DESIGN_FILENAME, manifest.experiment_design_sha256, "design") + design = published_path(output, DESIGN_FILENAME, manifest.experiment_design_sha256) + _check_sha(design, manifest.experiment_design_sha256, "design") _check_calibration(output / CALIBRATION_FILENAME, manifest.calibration_sha256) root = Path(experiment_root or manifest.experiment_root).resolve() _check_conversion(root, manifest) - recorded_masks = (manifest.masks or {}).get("folder") - masks = masks_root or recorded_masks - inputs = cls( + masks = masks_root or (manifest.masks or {}).get("folder") + return cls( output_folder=output, experiment_root=root, masks_root=None if masks is None else Path(masks).resolve(), manifest=manifest, - records=tuple(read_plane_histograms(output / OUTPUT_FILENAME)), + records=tuple(read_plane_histograms(table)), ) - inputs._check_files() - return inputs def fovs(self) -> tuple[Fov, ...]: """Return every position of the run, sorted by image path.""" @@ -329,10 +354,9 @@ def design(self) -> ExperimentDesign | None: expected = self.manifest.experiment_design_sha256 if expected is None: return None + path = published_path(self.output_folder, DESIGN_FILENAME, expected) try: - return read_confirmed_design( - self.output_folder / DESIGN_FILENAME, expected_sha256=expected - ).design + return read_confirmed_design(path, expected_sha256=expected).design except DesignCheckError as exc: raise RunInputsError( f"The design {DESIGN_FILENAME} cannot be used: {exc.issues[0].message} " @@ -506,69 +530,6 @@ def _check_conversion(root: Path, manifest: RunManifest) -> None: raise RunInputsError(problem) -def _changed_text(paths: list[str], what: str = "image") -> str: - """Say which files changed or went missing since the run.""" - where = "experiment_root=" if what == "image" else "masks_root=" - return ( - f"{len(paths)} {what}{'' if len(paths) == 1 else 's'} changed or missing since the " - f"run: {_shown(paths)}. Run preprocessing again on the experiment as it is (or, if the " - f"{what}s were only moved, give {where} where they are now)." - ) - - -def _shown(paths: tuple[str, ...] | list[str]) -> str: - """Name up to five paths, sorted.""" - return ", ".join(sorted(paths)[:5]) + (" and more" if len(paths) > 5 else "") - - -def _without_axes_text(paths: list[str]) -> str: - """Say which masks the run recorded without axes, and what to do.""" - return ( - f"The run recorded {plural(len(paths), 'mask')} without axes: " - f"{_shown(paths)}. A run before v0.4.0 wrote such a record for a mask whose header " - "puts X before Y; masks are never transposed. Save each such mask again with its " - "last two axes swapped (numpy.swapaxes(mask, -1, -2)), then run preprocessing again." - ) - - -def _unrecorded_text(files: UnrecordedFiles) -> str: - """Name the images and masks the run did not record, and what to do.""" - parts = [ - f"{plural(len(found), what)} ({_shown(found)})" - for what, found in (("image", files.images), ("mask", files.masks)) - if found - ] - masks = ( - " with its masks folder (--masks ; the label images of OME-Zarr images " - "are added)" - if files.masks - else "" - ) - exclude = ", or exclude the images (--exclude)" if files.images else "" - return ( - f"The experiment holds {' and '.join(parts)} that the preprocessing run did not " - "record, for example added since the run; they would be left out. Run preprocessing " - f"again on the experiment as it is{masks} (morphobricks-preprocessing run --output " - f"){exclude}." - ) - - -_ZARR_MISSING = ( - "The run holds OME-Zarr images, and the zarr package is not installed, so their label " - "images cannot be checked against the run (none is ever skipped). Install it with: " - 'pip install "morphobricks-preprocessing[zarr]"' -) - - -def _labels_unreadable_text(exc: NgffError) -> str: - """Say which OME-Zarr image's labels group cannot be read, and what to do.""" - return ( - f"The run cannot be checked: {exc}. Its label images may have changed since the " - "run; check the image's labels group, then run preprocessing again on the " - "experiment as it is." - ) - - def _mapping(entry: FileEntry) -> AxisMapping: """Return an image's own mapping as the run recorded it.""" return AxisMapping(entry.axes, tuple(entry.roles or ()), entry.singleton_roles) diff --git a/src/morphobricks_preprocessing/run_inputs_text.py b/src/morphobricks_preprocessing/run_inputs_text.py new file mode 100644 index 00000000..0f98d616 --- /dev/null +++ b/src/morphobricks_preprocessing/run_inputs_text.py @@ -0,0 +1,78 @@ +"""The texts :class:`.run_inputs.RunInputs` refuses a run with (moved from :mod:`.run_inputs`). + +Each says what changed or is missing since the run, and what to do. Nothing here reads or +writes files. +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING + +from .validation import plural + +if TYPE_CHECKING: + from .unrecorded import UnrecordedFiles + from .zarr_input import NgffError + + +def _changed_text(paths: list[str], what: str = "image") -> str: + """Say which files changed or went missing since the run.""" + where = "experiment_root=" if what == "image" else "masks_root=" + return ( + f"{len(paths)} {what}{'' if len(paths) == 1 else 's'} changed or missing since the " + f"run: {_shown(paths)}. Run preprocessing again on the experiment as it is (or, if the " + f"{what}s were only moved, give {where} where they are now)." + ) + + +def _shown(paths: tuple[str, ...] | list[str]) -> str: + """Name up to five paths, sorted.""" + return ", ".join(sorted(paths)[:5]) + (" and more" if len(paths) > 5 else "") + + +def _without_axes_text(paths: list[str]) -> str: + """Say which masks the run recorded without axes, and what to do.""" + return ( + f"The run recorded {plural(len(paths), 'mask')} without axes: " + f"{_shown(paths)}. A run before v0.4.0 wrote such a record for a mask whose header " + "puts X before Y; masks are never transposed. Save each such mask again with its " + "last two axes swapped (numpy.swapaxes(mask, -1, -2)), then run preprocessing again." + ) + + +def _unrecorded_text(files: UnrecordedFiles) -> str: + """Name the images and masks the run did not record, and what to do.""" + parts = [ + f"{plural(len(found), what)} ({_shown(found)})" + for what, found in (("image", files.images), ("mask", files.masks)) + if found + ] + masks = ( + " with its masks folder (--masks ; the label images of OME-Zarr images " + "are added)" + if files.masks + else "" + ) + exclude = ", or exclude the images (--exclude)" if files.images else "" + return ( + f"The experiment holds {' and '.join(parts)} that the preprocessing run did not " + "record, for example added since the run; they would be left out. Run preprocessing " + f"again on the experiment as it is{masks} (morphobricks-preprocessing run --output " + f"){exclude}." + ) + + +_ZARR_MISSING = ( + "The run holds OME-Zarr images, and the zarr package is not installed, so their label " + "images cannot be checked against the run (none is ever skipped). Install it with: " + 'pip install "morphobricks-preprocessing[zarr]"' +) + + +def _labels_unreadable_text(exc: NgffError) -> str: + """Say which OME-Zarr image's labels group cannot be read, and what to do.""" + return ( + f"The run cannot be checked: {exc}. Its label images may have changed since the " + "run; check the image's labels group, then run preprocessing again on the " + "experiment as it is." + ) diff --git a/src/morphobricks_preprocessing/sources.py b/src/morphobricks_preprocessing/sources.py index 4bf44ba9..0edc8e8f 100644 --- a/src/morphobricks_preprocessing/sources.py +++ b/src/morphobricks_preprocessing/sources.py @@ -12,7 +12,8 @@ from .axes import file_mapping_for from .image_names import position_id -from .manifest import LEGACY_MANIFEST_VERSION, RunManifest, source_fingerprint +from .inventory import source_signature +from .manifest import LEGACY_MANIFEST_VERSION, RunManifest from .metadata import inspect_image from .validation import ValidationIssue, error, plural @@ -30,8 +31,9 @@ def check_sources( The per-image checks every brick that uses ``plane_histograms.csv`` makes (MorphoBricks D86; formerly CellPatcher ``statistics.check_images`` and MorphoFeature ``histogram_fit``): each image is in the manifest; it is unchanged, by the size and - modification time :func:`source_fingerprint` gives (an OME-Zarr folder: its files' total - size and newest time); its recorded ``position_id`` is the one its name gives; with + modification time :func:`.inventory.source_signature` gives (an OME-Zarr folder: its + files' total size, newest time and inventory digest, issue #86); its recorded + ``position_id`` is the one its name gives; with ``identities``, its labels and ``position_id`` are the caller's; with ``image_axes``, the caller's axes give every axis the role the run confirmed for that image. A version 1 manifest (a run before v0.4.0, without the axis check) fails every image: such a run @@ -88,11 +90,11 @@ def report(code: str, path: str, detail: str = "") -> None: report("images_not_in_manifest", path) continue try: - fingerprint = source_fingerprint(root / path) + fingerprint = source_signature(root / path) except FileNotFoundError: report("images_missing", path) continue - if fingerprint != (entry.size, entry.mtime_ns): + if fingerprint != (entry.size, entry.mtime_ns, entry.inventory_sha256): report("images_changed", path) recorded = (*entry.labels, entry.position_id) if entry.position_id is not None and entry.position_id != position_id(path): diff --git a/tests/run_files.py b/tests/run_files.py index 4a0121fd..98c2c454 100644 --- a/tests/run_files.py +++ b/tests/run_files.py @@ -5,13 +5,18 @@ import sys from morphobricks_preprocessing.discovery_report import REPORT_FILENAME, REPORT_TEXT_FILENAME +from morphobricks_preprocessing.run.staging import STAGING_FOLDER REPORT_FILES = {REPORT_FILENAME, REPORT_TEXT_FILENAME} -def only_the_report(folder): - """True when a stopped run wrote nothing but its report (with status "stopped").""" - if not folder.is_dir() or {p.name for p in folder.iterdir()} != REPORT_FILES: +def only_the_report(folder, *, staged=False): + """True when a stopped run wrote nothing but its report (with status "stopped"). + + With ``staged``, the staging folder of the rows it read is there too (design v0.4 §1). + """ + expected = REPORT_FILES | ({STAGING_FOLDER} if staged else set()) + if not folder.is_dir() or {p.name for p in folder.iterdir()} != expected: return False return json.loads((folder / REPORT_FILENAME).read_text(encoding="utf-8"))["status"] == "stopped" @@ -28,8 +33,13 @@ def without_report(files): def outputs(folder): - """Return ``{name: bytes}`` of a folder's files, the report files left out.""" - return without_report({path.name: path.read_bytes() for path in folder.iterdir()}) + """Return ``{name: bytes}`` of a folder's files, the report files left out. + + The staging folder is left out too: a run that stops or fails keeps the rows it staged, + so the next run resumes them (design v0.4 §1); the staging tests check it. + """ + files = {path.name: path for path in folder.iterdir() if path.name != STAGING_FOLDER} + return without_report({name: path.read_bytes() for name, path in files.items()}) def finished_pid(): diff --git a/tests/test_cli_log.py b/tests/test_cli_log.py index 811da1c7..2b881760 100644 --- a/tests/test_cli_log.py +++ b/tests/test_cli_log.py @@ -78,7 +78,8 @@ def test_a_log_over_an_output_file_leaves_it_unchanged(clean, capsys, command, n "calibration.csv", "mask_relationships.json", ".morphobricks-preprocessing.lock", - ".run-x/log.txt", + ".publish/log.txt", + ".staging/log.txt", ], ) def test_a_log_named_as_another_output_is_refused(clean, capsys, name): diff --git a/tests/test_convert_integrity.py b/tests/test_convert_integrity.py index 3cf36be6..60cda0b6 100644 --- a/tests/test_convert_integrity.py +++ b/tests/test_convert_integrity.py @@ -132,12 +132,13 @@ def test_a_same_size_change_with_the_time_put_back_is_found_at_reuse(tmp_path): assert snapshot(copies) == before -def test_the_run_on_the_copies_warns_about_a_same_size_change(tmp_path): +def test_the_run_on_the_copies_stops_on_a_same_size_change(tmp_path): exp, copies = swapped_experiment(tmp_path / "exp"), tmp_path / "copies" assert convert_experiment(exp, SWAPPED, copies).written change_in_place(copies / "drug" / "r1" / "a.tif") run = run_experiment(copies, NAMED, output_folder=tmp_path / "out") found = {issue.code: issue for issue in run.issues} + assert not run.written and found["copy_changed_after_conversion"].is_error assert found["copy_changed_after_conversion"].paths == ("drug/r1/a.tif",) @@ -304,15 +305,19 @@ def test_an_ome_zarr_copy_holding_a_folder_link_counts_as_changed(tmp_path): assert not unchanged_content(zarr, expected) -def test_the_run_warning_about_an_added_copy_says_the_bricks_will_refuse_it(tmp_path): - # Review of PP-11c: the run goes on with the warning, but RunInputs refuses the run, so - # the warning must not suggest that analysing such a copy is an option. +def test_an_added_copy_stops_the_run_and_the_earlier_run_is_kept(tmp_path): + # Owner decision of 2 November 2026 (PP-12): an error, not a warning, since RunInputs + # would refuse the run; the earlier clean run stays as it was. exp, copies, out = swapped_experiment(tmp_path / "exp"), tmp_path / "copies", tmp_path / "o" assert convert_experiment(exp, SWAPPED, copies).written + assert run_experiment(copies, NAMED, output_folder=out).written + table = (out / "plane_histograms.csv").read_bytes() (copies / "ctrl" / "r1" / "n.tif").write_bytes((copies / "ctrl" / "r1" / "a.tif").read_bytes()) run = run_experiment(copies, NAMED, output_folder=out) found = {issue.code: issue for issue in run.issues}["copy_changed_after_conversion"] + assert not run.written and found.is_error assert found.paths == ("ctrl/r1/n.tif",) and "refuse" in found.message - assert "Convert the originals again" in found.action + assert "Convert the originals again into a new folder" in found.action + assert (out / "plane_histograms.csv").read_bytes() == table with pytest.raises(RunInputsError, match="ctrl/r1/n.tif"): RunInputs.open(out) diff --git a/tests/test_convert_run.py b/tests/test_convert_run.py index fcceb679..325ece80 100644 --- a/tests/test_convert_run.py +++ b/tests/test_convert_run.py @@ -338,17 +338,32 @@ def test_every_row_is_recomputed_and_equals_the_originals_rows(tmp_path): assert report["experiment_root_changed"] and report["design"] is not None -def test_the_run_warns_when_a_source_or_a_copy_changed_after_the_conversion(tmp_path): +def test_the_run_warns_when_a_source_changed_after_the_conversion(tmp_path): exp, copies = swapped_experiment(tmp_path / "exp"), tmp_path / "copies" assert convert_experiment(exp, SWAPPED, copies).written (exp / "ctrl" / "r1" / "a.tif").unlink() - copy = copies / "drug" / "r1" / "a.tif" - os.utime(copy, ns=(copy.stat().st_atime_ns, copy.stat().st_mtime_ns + 10**9)) run = run_experiment(copies, AxisMapping("ZYX", tuple("ZYX")), output_folder=tmp_path / "o") assert run.written found = {issue.code: issue for issue in run.issues} assert found["conversion_source_changed"].paths == ("ctrl/r1/a.tif",) - assert found["copy_changed_after_conversion"].paths == ("drug/r1/a.tif",) + assert not found["conversion_source_changed"].is_error + + +def test_a_copy_changed_after_the_conversion_stops_the_run_before_it_writes(tmp_path): + # Owner decision of 2 November 2026 (PP-12): the bricks would refuse the run anyway. + exp, copies, out = swapped_experiment(tmp_path / "exp"), tmp_path / "copies", tmp_path / "o" + assert convert_experiment(exp, SWAPPED, copies).written + copy = copies / "drug" / "r1" / "a.tif" + os.utime(copy, ns=(copy.stat().st_atime_ns, copy.stat().st_mtime_ns + 10**9)) + run = run_experiment(copies, AxisMapping("ZYX", tuple("ZYX")), output_folder=out) + assert not run.written and run.read == () + found = {issue.code: issue for issue in run.issues}["copy_changed_after_conversion"] + assert found.is_error and found.paths == ("drug/r1/a.tif",) + assert "Convert the originals again into a new folder" in found.action + assert sorted(p.name for p in out.iterdir()) == [ + "discovery_report.json", + "discovery_report.txt", + ] def test_an_unreadable_conversion_record_stops_the_run(tmp_path): diff --git a/tests/test_inventory.py b/tests/test_inventory.py new file mode 100644 index 00000000..1b7e7b16 --- /dev/null +++ b/tests/test_inventory.py @@ -0,0 +1,184 @@ +"""An OME-Zarr image's fingerprint covers every file of it (issue #86, PP-12). + +The fingerprint was the total size and the newest time: a chunk rewritten with the same +size whose new time stays below another file's newest time (or a chunk removed and another +of the same size added) left it unchanged, so a restart reused rows of pixels that no +longer exist. The inventory (each file's path, size and time) closes that gap. +""" + +import json +import os +import shutil + +import numpy as np +import pytest + +from morphobricks_preprocessing import ( + MANIFEST_FILENAME, + RunInputs, + check_sources, + prefill_mapping, + read_manifest, + run_experiment, +) +from morphobricks_preprocessing.fingerprint import Fingerprint +from morphobricks_preprocessing.inventory import source_fingerprint, source_signature +from morphobricks_preprocessing.run_inputs import RunInputsError +from zarr_tree import write_ngff + +pytest.importorskip("zarr") + +IMAGE = "ctrl/a.ome.zarr" + + +def two_chunks(path): + """Write a 2-plane image whose planes are one chunk each, of the same size.""" + data = np.stack([np.full((8, 8), 5, np.uint8), np.full((8, 8), 9, np.uint8)]) + image = write_ngff(path, data, "zyx", chunks=(1, 8, 8)) + first, second = chunk_files(image) + assert first.stat().st_size == second.stat().st_size + # Fixed times: the chunks are older than the metadata, so neither is the newest file. + for file in (p for p in image.rglob("*") if p.is_file()): + os.utime(file, ns=(3_000, 3_000)) + os.utime(first, ns=(1_000, 1_000)) + os.utime(second, ns=(2_000, 2_000)) + os.utime(image, ns=(3_000, 3_000)) + return image + + +def chunk_files(image): + files = sorted(p for p in (image / "0").iterdir() if not p.name.startswith(".")) + assert len(files) == 2, files + return files + + +def swap_chunks(image, first_time=1_500): + """Exchange the two chunks' bytes in place, keeping the size and the newest time.""" + first, second = chunk_files(image) + a, b = first.read_bytes(), second.read_bytes() + first.write_bytes(b) + second.write_bytes(a) + os.utime(first, ns=(first_time, first_time)) + os.utime(second, ns=(2_000, 2_000)) + os.utime(first.parent, ns=(3_000, 3_000)) + os.utime(image, ns=(3_000, 3_000)) + + +def test_a_rewritten_chunk_below_the_newest_time_changes_the_fingerprint(tmp_path): + # The issue's counterexample: same total size, same newest time. + image = two_chunks(tmp_path / "a.ome.zarr") + before = Fingerprint.of(image) + old_size, old_time = source_fingerprint(image) + swap_chunks(image) + assert source_fingerprint(image) == (old_size, old_time) # the old rule saw nothing + assert Fingerprint.of(image) != before + + +def test_a_chunk_removed_and_another_added_changes_the_fingerprint(tmp_path): + image = two_chunks(tmp_path / "a.ome.zarr") + before = Fingerprint.of(image) + first, _ = chunk_files(image) + moved = first.with_name("9.0.0") + shutil.copy2(first, moved) + first.unlink() + os.utime(first.parent, ns=(3_000, 3_000)) + assert source_fingerprint(image) == (before.size, before.mtime_ns) + assert Fingerprint.of(image) != before + + +def link_folder(link, target): + try: + link.symlink_to(target, target_is_directory=True) + except OSError: + pytest.skip("symbolic links need privileges here") + + +def test_a_chunk_behind_a_link_to_a_folder_changes_the_fingerprint(tmp_path): + # Review of PR #132: os.walk does not enter a linked folder by default, so a chunk + # changed behind the link (which the reader follows) left the fingerprint unchanged. + image, real = tmp_path / "a.ome.zarr", tmp_path / "chunks" + (image / "0").mkdir(parents=True) + (image / "zarr.json").write_text("{}", encoding="utf-8") + real.mkdir() + (real / "0").write_bytes(b"ab") + link_folder(image / "0" / "0", real) + before = Fingerprint.of(image) + os.utime(real / "0", ns=(1_000, 1_000)) + assert Fingerprint.of(image) != before + + +def test_links_that_reach_a_folder_twice_are_refused(tmp_path): + image = tmp_path / "a.ome.zarr" + (image / "0").mkdir(parents=True) + link_folder(image / "0" / "loop", image) + with pytest.raises(OSError, match="twice"): + source_signature(image) + + +def test_an_unchanged_store_keeps_its_fingerprint_and_labels_are_left_out(tmp_path): + image = two_chunks(tmp_path / "a.ome.zarr") + before = Fingerprint.of(image) + assert before.inventory is not None and len(before.inventory) == 64 + assert Fingerprint.of(image) == before + labels = image / "labels" / "cells" + labels.mkdir(parents=True) + (labels / "chunk").write_bytes(b"1234") + os.utime(image, ns=(3_000, 3_000)) + assert Fingerprint.of(image) == before + + +def test_a_file_has_no_inventory(tmp_path): + path = tmp_path / "a.tif" + path.write_bytes(b"1234") + assert source_signature(path)[2] is None + assert Fingerprint.of(path).inventory is None + + +@pytest.fixture +def zarr_run(tmp_path): + root = tmp_path / "exp" + image = two_chunks(root / IMAGE) + output = tmp_path / "out" + result = run_experiment(root, prefill_mapping("ZYX"), output_folder=output) + assert result.written, result.issues + return root, image, output + + +def maxima(output): + inputs = RunInputs.open(output) + return [r.histograms[0].pixel_max for r in inputs.histograms()] + + +def test_a_restart_recomputes_an_image_whose_chunk_changed(zarr_run): + root, image, output = zarr_run + entry = read_manifest(output / MANIFEST_FILENAME).files[0] + assert entry.inventory_sha256 == Fingerprint.of(image).inventory + assert maxima(output) == [5, 9] + swap_chunks(image) + result = run_experiment(root, output_folder=output) + assert result.written, result.issues + assert result.changed == (IMAGE,) and result.read == (IMAGE,) + assert maxima(output) == [9, 5] # the rows describe the pixels as they are now + + +def test_bricks_refuse_an_image_whose_chunk_changed(zarr_run): + root, image, output = zarr_run + manifest = read_manifest(output / MANIFEST_FILENAME) + swap_chunks(image) + found = {issue.code: issue.paths for issue in check_sources(manifest, root, [IMAGE])} + assert found == {"images_changed": (IMAGE,)} + with pytest.raises(RunInputsError, match="changed"): + RunInputs.open(output).read_plane(IMAGE, 0, 0) + + +def test_an_image_recorded_without_inventory_is_read_again(zarr_run): + # A manifest written before the inventory was recorded (v0.4.0rc3): its OME-Zarr images + # are not known to be unchanged, so their rows are recomputed, never reused. + root, _, output = zarr_run + path = output / MANIFEST_FILENAME + data = json.loads(path.read_text(encoding="utf-8")) + del data["files"][0]["inventory_sha256"] + path.write_text(json.dumps(data), encoding="utf-8") + result = run_experiment(root, output_folder=output) + assert result.written, result.issues + assert result.changed == (IMAGE,) diff --git a/tests/test_publication.py b/tests/test_publication.py new file mode 100644 index 00000000..0b9f2c75 --- /dev/null +++ b/tests/test_publication.py @@ -0,0 +1,311 @@ +"""Crash-consistent publication (PP-12, owner decision of 2 November 2026; contract §9). + +A run's outputs are one generation, made current by one atomic step (the manifest's +replacement): any reader through the manifest sees the old or the new generation, a plain +reader of plane_histograms.csv one whole table, never a mix; a kill at any step is +finished or discarded by the next holder of the lock. +""" + +import json +import os +import shutil +import signal +import subprocess +import sys +from pathlib import Path + +import numpy as np +import pytest +from morphobricks_normalization.csv_io import OUTPUT_FILENAME, read_plane_histograms + +from morphobricks_preprocessing import ( + MANIFEST_FILENAME, + RunInputs, + prefill_mapping, + read_manifest, + run_experiment, +) +from morphobricks_preprocessing import run as run_module +from morphobricks_preprocessing.manifest import file_sha256 +from morphobricks_preprocessing.run.generation import ( + PUBLICATION_RECORD, + PUBLISH_FOLDER, + PublicationIncompleteError, +) +from morphobricks_preprocessing.run.lock import output_lock, remove_stale_lock +from morphobricks_preprocessing.testing import _confirmed_design +from run_files import outputs +from tiff_tree import write_data + +ZYX = prefill_mapping("ZYX") +IMAGES = ["ctrl/r1/a.tif", "ctrl/r1/b.tif", "drug/r1/a.tif"] +MASK_REPORT = "mask_relationships.json" + + +def stack(seed): + return np.random.default_rng(seed).integers(0, 255, (2, 8, 8), dtype=np.uint8) + + +NEXT = {"mode": "full", "bin_count": 128} +"""The run that publishes generation B: every row recomputed with other bins.""" + + +def first_generation(base): + """Write an experiment and its run (generation A); :data:`NEXT` gives generation B. + + The images stay as they are, so both generations can be opened; B has other bins. + """ + root, out = base / "exp", base / "out" + for seed, rel in enumerate(IMAGES): + write_data(root / rel, stack(seed), "ZYX") + assert run_experiment(root, ZYX, output_folder=out, design=_confirmed_design(root)).written + (out / MASK_REPORT).write_text("{}\n", encoding="utf-8") # a full run removes it + return root, out + + +def same(rows, other): + """True when two lists of rows hold the same planes with the same histograms.""" + + def key(records): + return [(r.plane_id, [h.counts.tolist() for h in r.histograms]) for r in records] + + return key(rows) == key(other) + + +def consistent(out): + """Return the generation's table as RunInputs reads it, checking it is one generation.""" + inputs = RunInputs.open(out) + manifest = inputs.manifest + design = inputs.design() + assert design is not None + return manifest.csv_sha256, inputs.histograms() + + +# Failures inside the process ------------------------------------------------------------- + + +@pytest.mark.parametrize("failure", ["csv", "hash", "manifest", "record", "commit"]) +def test_a_failure_before_the_commit_keeps_the_earlier_generation(tmp_path, monkeypatch, failure): + root, out = first_generation(tmp_path) + before = outputs(out) + generation = run_module.generation + original_replace, original_hash = os.replace, generation.file_sha256 + + def broken(*args, **kwargs): + raise OSError("simulated failure") + + def broken_commit(source, destination): + if Path(destination) == out / MANIFEST_FILENAME: + raise PermissionError("simulated failure") + return original_replace(source, destination) + + def broken_hash(path): + if Path(path).parent.name == PUBLISH_FOLDER: + raise OSError("simulated failure") + return original_hash(path) + + with monkeypatch.context() as patch: + target = { + "csv": (run_module.pipeline, "write_plane_histograms", broken), + "hash": (generation, "file_sha256", broken_hash), + "manifest": (run_module.pipeline, "write_manifest", broken), + "record": (generation, "write_text_atomic", broken), + "commit": (os, "replace", broken_commit), + }[failure] + patch.setattr(*target) + with pytest.raises(OSError, match="simulated failure"): + run_experiment(root, output_folder=out, **NEXT) + assert outputs(out) == before + assert not (out / PUBLISH_FOLDER).exists() + retried = run_experiment(root, output_folder=out, **NEXT) + assert retried.written + assert read_manifest(retried.manifest_path).csv_sha256 == file_sha256(retried.csv_path) + + +def test_an_interruption_at_the_commit_keeps_the_earlier_generation(tmp_path, monkeypatch): + root, out = first_generation(tmp_path) + before = outputs(out) + original = os.replace + + def interrupted(source, destination): + if Path(destination) == out / MANIFEST_FILENAME: + raise KeyboardInterrupt + return original(source, destination) + + monkeypatch.setattr(os, "replace", interrupted) + with pytest.raises(KeyboardInterrupt): + run_experiment(root, output_folder=out, **NEXT) + assert outputs(out) == before and not (out / PUBLISH_FOLDER).exists() + + +def test_a_failure_after_the_commit_is_finished_by_the_next_run(tmp_path, monkeypatch): + # For example a table held open by another program on Windows. + root, out = first_generation(tmp_path) + old_table = (out / OUTPUT_FILENAME).read_bytes() + original = os.replace + + def table_in_use(source, destination): + if Path(destination) == out / OUTPUT_FILENAME: + raise PermissionError("simulated table in use") + return original(source, destination) + + with monkeypatch.context() as patch: + patch.setattr(os, "replace", table_in_use) + with pytest.raises(PublicationIncompleteError, match="next run finishes"): + run_experiment(root, output_folder=out, **NEXT) + # The new generation is current: RunInputs reads its table from the publication folder; + # a plain reader still reads the earlier, whole table. + manifest = read_manifest(out / MANIFEST_FILENAME) + assert (out / OUTPUT_FILENAME).read_bytes() == old_table + assert file_sha256(out / PUBLISH_FOLDER / OUTPUT_FILENAME) == manifest.csv_sha256 + sha, rows = consistent(out) + assert sha == manifest.csv_sha256 + assert same(rows, read_plane_histograms(out / PUBLISH_FOLDER / OUTPUT_FILENAME)) + # Whatever takes the lock next finishes the publication first. + with output_lock(out): + pass + assert not (out / PUBLISH_FOLDER).exists() and not (out / MASK_REPORT).exists() + assert file_sha256(out / OUTPUT_FILENAME) == manifest.csv_sha256 + again = run_experiment(root, output_folder=out) + assert again.written and again.read == () + + +def test_an_unfinished_uncommitted_publication_is_discarded(tmp_path): + root, out = first_generation(tmp_path) + before = outputs(out) + leftover = out / PUBLISH_FOLDER + leftover.mkdir() + (leftover / OUTPUT_FILENAME).write_text("not a table", encoding="utf-8") + (leftover / PUBLICATION_RECORD).write_text( + json.dumps({"publication_version": "1", "files": {MANIFEST_FILENAME: "0" * 64}}), + encoding="utf-8", + ) + consistent(out) # readers ignore it: the manifest in place is the earlier one + with output_lock(out): + pass + assert not leftover.exists() and outputs(out) == before + + +def test_a_cleanup_failure_does_not_fail_a_published_run(tmp_path, monkeypatch, caplog): + root, out = first_generation(tmp_path) + original = shutil.rmtree + + def broken(path, *args, **kwargs): + if Path(path).name == PUBLISH_FOLDER and (Path(path) / MANIFEST_FILENAME).exists() is False: + raise PermissionError("simulated folder in use") + return original(path, *args, **kwargs) + + monkeypatch.setattr(shutil, "rmtree", broken) + result = run_experiment(root, output_folder=out, **NEXT) + assert result.written + assert read_manifest(result.manifest_path).csv_sha256 == file_sha256(result.csv_path) + assert "Cannot remove the publication folder" in caplog.text + + +# Killed between any two steps (POSIX) ---------------------------------------------------- + +CHILD = """ +import contextlib, json, os, shutil, signal, sys +from pathlib import Path +from morphobricks_preprocessing import run_experiment +from morphobricks_preprocessing.run import pipeline + +root, out, kill_at = Path(sys.argv[1]), Path(sys.argv[2]), int(sys.argv[3]) +state = {"calls": 0, "active": False} + +def step(function): + def wrapped(*args, **kwargs): + if state["active"]: + state["calls"] += 1 + if state["calls"] == kill_at: + os.kill(os.getpid(), signal.SIGKILL) + return function(*args, **kwargs) + return wrapped + +# Every step that changes a file during the publication: writes (each an os.replace of +# its temporary file), the commit, the moves, the removals and the clean-up. +os.replace = step(os.replace) +os.unlink = step(os.unlink) +shutil.rmtree = step(shutil.rmtree) +Path.unlink = step(Path.unlink) +original = pipeline.staged_generation + +@contextlib.contextmanager +def watched(*args, **kwargs): + state["active"] = True + with original(*args, **kwargs) as staged: + yield staged + +pipeline.staged_generation = watched +result = run_experiment(root, output_folder=out, mode="full", bin_count=128) +print(json.dumps({"calls": state["calls"], "written": result.written})) +""" + + +def child(base, kill_at): + script = base.parent / "child.py" + script.write_text(CHILD, encoding="utf-8") + env = {**os.environ, "PYTHONPATH": os.pathsep.join(sys.path)} + return subprocess.run( + [sys.executable, str(script), str(base / "exp"), str(base / "out"), str(kill_at)], + capture_output=True, + text=True, + env=env, + check=False, + ) + + +@pytest.mark.skipif(os.name != "posix", reason="SIGKILL is POSIX only") +def test_a_kill_between_any_two_publication_steps_leaves_one_generation(tmp_path): + template = tmp_path / "template" + first_generation(template) + old_table = (template / "out" / OUTPUT_FILENAME).read_bytes() + old_sha = file_sha256(template / "out" / OUTPUT_FILENAME) + + def fresh(name): + base = tmp_path / name + shutil.copytree(template, base, symlinks=True) + return base + + complete = fresh("complete") + done = child(complete, 0) + assert done.returncode == 0, done.stderr + steps = json.loads(done.stdout.splitlines()[-1])["calls"] + new_table = (complete / "out" / OUTPUT_FILENAME).read_bytes() + new_sha = file_sha256(complete / "out" / OUTPUT_FILENAME) + assert new_sha != old_sha and steps >= 8 + seen = set() + for kill_at in range(1, steps + 1): + base = fresh(f"kill-{kill_at}") + out = base / "out" + killed = child(base, kill_at) + assert killed.returncode == -signal.SIGKILL, (kill_at, killed.stdout, killed.stderr) + # A plain reader: one whole table of one generation. + plain = (out / OUTPUT_FILENAME).read_bytes() + assert plain in (old_table, new_table), kill_at + assert read_plane_histograms(out / OUTPUT_FILENAME) + # A reader through the manifest: exactly one generation, the old or the new one. + sha, rows = consistent_at(out, base / "exp") + assert sha in (old_sha, new_sha), kill_at + expected = template / "out" if sha == old_sha else complete / "out" + assert same(rows, read_plane_histograms(expected / OUTPUT_FILENAME)), kill_at + if sha == old_sha: # not committed: nothing in the folder changed + assert plain == old_table and (out / MASK_REPORT).exists(), kill_at + seen.add("old" if sha == old_sha else "new") + # The next holder of the lock finishes or discards it; the folder is then whole. + remove_stale_lock(out) + with output_lock(out): + pass + assert not (out / PUBLISH_FOLDER).exists(), kill_at + manifest = read_manifest(out / MANIFEST_FILENAME) + assert manifest.csv_sha256 == file_sha256(out / OUTPUT_FILENAME) == sha, kill_at + assert (out / MASK_REPORT).exists() == (sha == old_sha), kill_at + again = run_experiment(base / "exp", output_folder=out) + assert again.written, (kill_at, again.issues) + assert seen == {"old", "new"} + + +def consistent_at(out, root): + inputs = RunInputs.open(out, experiment_root=root) + assert inputs.design() is not None + return inputs.manifest.csv_sha256, inputs.histograms() diff --git a/tests/test_resume.py b/tests/test_resume.py new file mode 100644 index 00000000..05c09de4 --- /dev/null +++ b/tests/test_resume.py @@ -0,0 +1,322 @@ +"""Per-file staging and resume (design v0.4 §1, PP-12) and source changes during a run (#90). + +Each file's rows are staged as soon as it is read. A run killed part-way (a crash, a kill, +a lost Colab session) resumes on the next run from the staged rows of files that are as +they were, and recomputes only the rest; staged rows whose image, settings or version +changed are discarded and reported, never reused. A file that changes while the run checks +or reads it is never published. +""" + +import json +import os +import signal +import subprocess +import sys + +import numpy as np +import pytest +from morphobricks_normalization.csv_io import OUTPUT_FILENAME + +from morphobricks_preprocessing import PlaneReadError, RunInputs, prefill_mapping, run_experiment +from morphobricks_preprocessing import run as run_module +from morphobricks_preprocessing.discovery_report import REPORT_FILENAME +from morphobricks_preprocessing.run.lock import remove_stale_lock +from morphobricks_preprocessing.run.staging import STAGING_FOLDER +from run_files import outputs +from tiff_tree import write_data + +ZYX = prefill_mapping("ZYX") +IMAGES = ["ctrl/r1/a.tif", "ctrl/r1/b.tif", "ctrl/r2/a.tif", "drug/r1/a.tif", "drug/r2/a.tif"] + + +def stack(seed): + return np.random.default_rng(seed).integers(0, 255, (2, 8, 8), dtype=np.uint8) + + +def change(path, seed=99): + before = path.stat().st_mtime_ns + write_data(path, stack(seed), "ZYX") + os.utime(path, ns=(before + 10**9, before + 10**9)) + + +@pytest.fixture +def experiment(tmp_path): + root = tmp_path / "exp" + for seed, rel in enumerate(IMAGES): + write_data(root / rel, stack(seed), "ZYX") + return root, tmp_path / "out" + + +def reference_table(tmp_path, root): + """The table of an uninterrupted run on the experiment as it is.""" + result = run_experiment(root, ZYX, output_folder=tmp_path / "reference") + assert result.written + return result.csv_path.read_bytes() + + +def report(out): + return json.loads((out / REPORT_FILENAME).read_text(encoding="utf-8")) + + +KILLED_RUN = """ +import os, signal, sys +from pathlib import Path +from morphobricks_preprocessing import prefill_mapping, run_experiment +from morphobricks_preprocessing.run import resume + +root, out, kill_at = Path(sys.argv[1]), Path(sys.argv[2]), int(sys.argv[3]) +read = resume._read_file +calls = [] + +def killing(*args, **kwargs): + calls.append(args[1]) + if len(calls) == kill_at: + os.kill(os.getpid(), signal.SIGKILL) + return read(*args, **kwargs) + +resume._read_file = killing +run_experiment(root, prefill_mapping("ZYX"), output_folder=out) +""" + + +def killed_run(tmp_path, root, out, kill_at): + """Run in another process and kill it as it starts reading file number ``kill_at``.""" + script = tmp_path / "killed_run.py" + script.write_text(KILLED_RUN, encoding="utf-8") + env = {**os.environ, "PYTHONPATH": os.pathsep.join(sys.path)} + done = subprocess.run( + [sys.executable, str(script), str(root), str(out), str(kill_at)], + capture_output=True, + text=True, + env=env, + check=False, + ) + assert done.returncode == -signal.SIGKILL, done.stderr + remove_stale_lock(out) # the user confirms the killed run is gone + + +@pytest.mark.skipif(os.name != "posix", reason="SIGKILL is POSIX only") +def test_a_killed_run_resumes_from_the_staged_rows(tmp_path, experiment): + root, out = experiment + killed_run(tmp_path, root, out, kill_at=4) + assert not (out / OUTPUT_FILENAME).exists() # nothing was published + assert len(list((out / STAGING_FOLDER).glob("*.json"))) == 3 + result = run_experiment(root, ZYX, output_folder=out) + assert result.written + assert result.resumed == tuple(IMAGES[:3]) and result.read == tuple(IMAGES[3:]) + assert result.csv_path.read_bytes() == reference_table(tmp_path, root) + assert not (out / STAGING_FOLDER).exists() + found = report(out) + assert found["resumed"] == IMAGES[:3] and sorted(found["recomputed"]) == IMAGES[3:] + assert "Resumed: 3 images" in (out / "discovery_report.txt").read_text(encoding="utf-8") + RunInputs.open(out) + + +@pytest.mark.skipif(os.name != "posix", reason="SIGKILL is POSIX only") +def test_a_resumed_restart_reads_only_the_files_it_must(tmp_path, experiment): + root, out = experiment + assert run_experiment(root, ZYX, output_folder=out).written + for rel in IMAGES[1:4]: + change(root / rel, seed=50 + len(rel)) + killed_run(tmp_path, root, out, kill_at=3) # b and r2/a staged, killed at drug/r1/a + result = run_experiment(root, output_folder=out) + assert result.written and result.unchanged == (IMAGES[0], IMAGES[4]) + assert result.resumed == tuple(IMAGES[1:3]) and result.read == (IMAGES[3],) + assert result.csv_path.read_bytes() == reference_table(tmp_path, root) + + +@pytest.mark.skipif(os.name != "posix", reason="SIGKILL is POSIX only") +def test_staged_rows_of_an_image_changed_since_are_discarded(tmp_path, experiment): + root, out = experiment + killed_run(tmp_path, root, out, kill_at=4) + change(root / IMAGES[1]) + result = run_experiment(root, ZYX, output_folder=out) + assert result.written + assert result.resumed == (IMAGES[0], IMAGES[2]) + assert result.read == (IMAGES[1], *IMAGES[3:]) + assert report(out)["staged_discarded"] == { + IMAGES[1]: "the image changed since its rows were staged" + } + assert result.csv_path.read_bytes() == reference_table(tmp_path, root) + + +@pytest.mark.skipif(os.name != "posix", reason="SIGKILL is POSIX only") +@pytest.mark.parametrize( + ("given", "why"), + [ + ({"bin_count": 128}, "its axis roles, labels, folder or row settings changed"), + ({"mode": "full"}, "a full run reuses no staged row"), + ], +) +def test_staged_rows_are_not_reused_with_other_settings_or_in_full_mode( + tmp_path, experiment, given, why +): + root, out = experiment + killed_run(tmp_path, root, out, kill_at=3) + result = run_experiment(root, ZYX, output_folder=out, **given) + assert result.written and result.resumed == () and result.read == tuple(IMAGES) + discarded = report(out)["staged_discarded"] + assert sorted(discarded) == IMAGES[:2] + assert all(reason.startswith(why) for reason in discarded.values()) + + +@pytest.mark.skipif(os.name != "posix", reason="SIGKILL is POSIX only") +def test_damaged_staged_rows_are_discarded_and_reported(tmp_path, experiment): + root, out = experiment + killed_run(tmp_path, root, out, kill_at=3) + (rows,) = sorted((out / STAGING_FOLDER).glob("*.csv"))[:1] + rows.write_text(rows.read_text(encoding="utf-8").replace("ctrl", "drug"), encoding="utf-8") + (half,) = sorted((out / STAGING_FOLDER).glob("*.json"))[1:2] + half.unlink() # a run killed between the rows file and its record: never staged + result = run_experiment(root, ZYX, output_folder=out) + assert result.written and result.resumed == () and result.read == tuple(IMAGES) + (reason,) = report(out)["staged_discarded"].values() + assert reason.startswith("its staged rows could not be read") + + +def test_a_stopped_run_keeps_its_staged_rows_and_a_published_run_removes_them( + experiment, monkeypatch +): + root, out = experiment + read = run_module.resume._read_file + + def unreadable(path, rel, *args): + if rel == IMAGES[-1]: + raise PlaneReadError("simulated") + return read(path, rel, *args) + + with monkeypatch.context() as patch: + patch.setattr(run_module.resume, "_read_file", unreadable) + stopped = run_experiment(root, ZYX, output_folder=out) + assert not stopped.written and stopped.read == tuple(IMAGES[:-1]) + assert (out / STAGING_FOLDER).is_dir() + result = run_experiment(root, ZYX, output_folder=out) + assert result.written and result.read == (IMAGES[-1],) + assert result.resumed == tuple(IMAGES[:-1]) + assert not (out / STAGING_FOLDER).exists() + + +# Issue #90: sources that change while the run checks or reads them ------------------------- + + +def test_an_image_changed_while_it_is_read_is_never_published(tmp_path, experiment, monkeypatch): + root, out = experiment + assert run_experiment(root, ZYX, output_folder=out).written + before = outputs(out) + change(root / IMAGES[2]) + read = run_module.resume._read_file + + def written_meanwhile(path, rel, *args): + rows = read(path, rel, *args) + if rel == IMAGES[2]: + change(path, seed=77) # an acquisition still writing the file + return rows + + with monkeypatch.context() as patch: + patch.setattr(run_module.resume, "_read_file", written_meanwhile) + result = run_experiment(root, output_folder=out) + assert not result.written + (found,) = [i for i in result.issues if i.code == "source_changed_during_run"] + assert found.is_error and found.paths == (IMAGES[2],) + assert outputs(out) == before # the earlier generation is intact + assert not list((out / STAGING_FOLDER).glob("*.json")) # its rows were not staged + again = run_experiment(root, output_folder=out) + assert again.written and again.read == (IMAGES[2],) + assert again.csv_path.read_bytes() == reference_table(tmp_path, root) + + +def test_an_image_changed_after_it_was_read_stops_the_publication(experiment, monkeypatch): + # Every image is fingerprinted again just before the commit, the reused ones included. + root, out = experiment + assert run_experiment(root, ZYX, output_folder=out).written + before = outputs(out) + change(root / IMAGES[3]) + changed_since = run_module.pipeline.changed_since + + def meanwhile(entries, where): + change(root / IMAGES[0], seed=5) # a reused image + change(root / IMAGES[3], seed=6) # the image just read and staged + return changed_since(entries, where) + + with monkeypatch.context() as patch: + patch.setattr(run_module.pipeline, "changed_since", meanwhile) + result = run_experiment(root, output_folder=out) + assert not result.written + (found,) = [i for i in result.issues if i.code == "source_changed_during_run"] + assert found.paths == (IMAGES[0], IMAGES[3]) + assert outputs(out) == before and not (out / ".publish").exists() + again = run_experiment(root, output_folder=out) + assert again.written and again.read == (IMAGES[0], IMAGES[3]) and again.resumed == () + assert report(out)["staged_discarded"] == { + IMAGES[3]: "the image changed since its rows were staged" + } + + +def test_an_image_changed_after_its_header_was_read_is_not_read(experiment, monkeypatch): + root, out = experiment + entries = run_module.steps.file_entries + + def after_the_header(pre, *args): + change(root / IMAGES[1]) # between the header check and the manifest entry + return entries(pre, *args) + + with monkeypatch.context() as patch: + patch.setattr(run_module.steps, "file_entries", after_the_header) + result = run_experiment(root, ZYX, output_folder=out) + assert not result.written and IMAGES[1] not in result.read + (found,) = [i for i in result.issues if i.code == "source_changed_during_run"] + assert found.paths == (IMAGES[1],) + + +def test_a_chunk_changed_while_an_ome_zarr_image_is_read_is_never_published(tmp_path, monkeypatch): + pytest.importorskip("zarr") + from zarr_tree import write_ngff + + root, out = tmp_path / "exp", tmp_path / "out" + data = np.stack([np.full((8, 8), 5, np.uint8), np.full((8, 8), 9, np.uint8)]) + image = write_ngff(root / "ctrl" / "a.ome.zarr", data, "zyx", chunks=(1, 8, 8)) + read = run_module.resume._read_file + + def chunk_rewritten(path, rel, *args): + rows = read(path, rel, *args) + chunk = sorted(p for p in (image / "0").iterdir() if not p.name.startswith("."))[0] + stat = chunk.stat() + chunk.write_bytes(chunk.read_bytes()) # same bytes and size, another time + # 1 ms later: NTFS keeps times in 100 ns steps, so 1 ns would be rounded away. + os.utime(chunk, ns=(stat.st_atime_ns, stat.st_mtime_ns + 10**6)) + return rows + + monkeypatch.setattr(run_module.resume, "_read_file", chunk_rewritten) + result = run_experiment(root, ZYX, output_folder=out) + assert not result.written + assert [i.code for i in result.issues if i.is_error] == ["source_changed_during_run"] + + +@pytest.mark.skipif(os.name != "posix", reason="SIGKILL is POSIX only") +def test_the_summary_names_the_resumed_images(tmp_path, experiment): + from morphobricks_preprocessing.report import run_summary + + root, out = experiment + killed_run(tmp_path, root, out, kill_at=3) + result = run_experiment(root, ZYX, output_folder=out) + assert "Resumed (rows staged by an interrupted run): 2 images" in run_summary(result) + + +@pytest.mark.skipif(os.name != "posix", reason="SIGKILL is POSIX only") +def test_a_discarded_record_is_reported_by_its_image_and_its_true_reason(tmp_path, experiment): + # Review of PR #132: another pixel type was reported as changed roles or settings, and + # unreadable rows by the record's hashed file name instead of the image. + root, out = experiment + killed_run(tmp_path, root, out, kill_at=3) + first, second = sorted((out / STAGING_FOLDER).glob("*.json")) + record = json.loads(first.read_text(encoding="utf-8")) + record["key"]["dtype"] = "uint16" + first.write_text(json.dumps(record), encoding="utf-8") + rows = second.with_suffix(".csv") + rows.write_text(rows.read_text(encoding="utf-8") + "\n", encoding="utf-8") + named = json.loads(second.read_text(encoding="utf-8"))["relative_path"] + result = run_experiment(root, ZYX, output_folder=out) + assert result.written and result.resumed == () + discarded = report(out)["staged_discarded"] + assert discarded[record["relative_path"]].startswith("the experiment's pixel type changed") + assert discarded[named].startswith("its staged rows could not be read") diff --git a/tests/test_run.py b/tests/test_run.py index d126274a..95d3c0c5 100644 --- a/tests/test_run.py +++ b/tests/test_run.py @@ -1,7 +1,6 @@ import hashlib import json import os -import shutil from dataclasses import replace from pathlib import Path @@ -29,8 +28,7 @@ ) from morphobricks_preprocessing import planes as planes_module from morphobricks_preprocessing import run as run_module -from morphobricks_preprocessing.manifest import file_sha256 -from run_files import only_the_report, outputs +from run_files import only_the_report from simulation import simulate_plane from tiff_tree import write_data, write_file @@ -574,129 +572,6 @@ def test_first_run_with_errors_writes_only_the_report(experiment): assert not result.written and only_the_report(result.output_folder) -@pytest.mark.parametrize("failure", ["csv", "hash", "manifest", "backup"]) -def test_preparing_outputs_preserves_earlier_files_on_failure(experiment, monkeypatch, failure): - first = run_experiment(experiment, ZYX) - before = {path.name: path.read_bytes() for path in (first.csv_path, first.manifest_path)} - touch(experiment / IMAGES[0], stack(99)) - original_hash = run_module.pipeline.file_sha256 - - def broken_csv(*args, **kwargs): - raise OSError("simulated CSV failure") - - def broken_hash(path): - if Path(path).parent != first.output_folder: - raise OSError("simulated hash failure") - return original_hash(path) - - def broken_manifest(*args, **kwargs): - raise OSError("simulated manifest failure") - - def broken_backup(*args, **kwargs): - raise OSError("simulated backup failure") - - with monkeypatch.context() as patch: - if failure == "csv": - patch.setattr(run_module.pipeline, "write_plane_histograms", broken_csv) - elif failure == "hash": - patch.setattr(run_module.pipeline, "file_sha256", broken_hash) - elif failure == "manifest": - patch.setattr(run_module.pipeline, "write_manifest", broken_manifest) - else: - patch.setattr(shutil, "copy2", broken_backup) - with pytest.raises(OSError, match="simulated"): - run_experiment(experiment, ZYX) - assert outputs(first.output_folder) == before - retried = run_experiment(experiment, ZYX) - assert retried.written and retried.changed == (IMAGES[0],) - - -@pytest.mark.parametrize("previous", ["none", "both", "csv", "manifest"]) -@pytest.mark.parametrize("failed_name", [OUTPUT_FILENAME, MANIFEST_FILENAME]) -def test_publishing_outputs_restores_the_previous_state( - experiment, monkeypatch, previous, failed_name -): - first = run_experiment(experiment, ZYX) - for path, keep in ((first.csv_path, "csv"), (first.manifest_path, "manifest")): - if previous not in ("both", keep): - path.unlink() - unrelated = first.output_folder / "notes.txt" - unrelated.write_text("keep this file", encoding="utf-8") - before = outputs(first.output_folder) - touch(experiment / IMAGES[0], stack(99)) - original_replace = os.replace - failed = False - - def broken_replace(source, destination): - nonlocal failed - destination = Path(destination) - if destination == first.output_folder / failed_name and not failed: - failed = True - raise PermissionError("simulated publication failure") - return original_replace(source, destination) - - with monkeypatch.context() as patch: - patch.setattr(os, "replace", broken_replace) - with pytest.raises(PermissionError, match="simulated publication failure"): - run_experiment(experiment, ZYX) - assert failed - assert outputs(first.output_folder) == before - retried = run_experiment(experiment, ZYX) - assert retried.written - assert read_manifest(retried.manifest_path).csv_sha256 == file_sha256(retried.csv_path) - assert unrelated.read_text(encoding="utf-8") == "keep this file" - - -def test_interrupted_publication_restores_earlier_output(experiment, monkeypatch): - first = run_experiment(experiment, ZYX) - before = outputs(first.output_folder) - touch(experiment / IMAGES[0], stack(99)) - original_replace = os.replace - - def interrupted(source, destination): - if Path(destination) == first.manifest_path: - raise KeyboardInterrupt - return original_replace(source, destination) - - monkeypatch.setattr(os, "replace", interrupted) - with pytest.raises(KeyboardInterrupt): - run_experiment(experiment, ZYX) - assert outputs(first.output_folder) == before - - -def test_failed_rollback_retains_backups_and_reports_their_location(experiment, monkeypatch): - first = run_experiment(experiment, ZYX) - before = outputs(first.output_folder) - touch(experiment / IMAGES[0], stack(99)) - original_replace = os.replace - - def broken_replace(source, destination): - if Path(destination) == first.manifest_path or Path(source).parent.name == "previous": - raise PermissionError("simulated locked output") - return original_replace(source, destination) - - monkeypatch.setattr(os, "replace", broken_replace) - with pytest.raises(RuntimeError, match="could not be fully restored") as excinfo: - run_experiment(experiment, ZYX) - (staged,) = first.output_folder.glob(".run-*") - backup = staged / "previous" - assert str(backup) in str(excinfo.value) - assert outputs(backup) == before - - -def test_cleanup_failure_does_not_report_completed_output_as_failed( - experiment, monkeypatch, caplog -): - def broken_cleanup(path): - raise PermissionError("simulated temporary folder in use") - - monkeypatch.setattr(shutil, "rmtree", broken_cleanup) - result = run_experiment(experiment, ZYX) - assert result.written - assert read_manifest(result.manifest_path).csv_sha256 == file_sha256(result.csv_path) - assert "Cannot remove temporary output folder" in caplog.text - - def test_output_inside_experiment_is_refused(experiment): result = run_experiment(experiment, ZYX, output_folder=experiment / "results") assert "output_inside_experiment" in codes(result.issues) @@ -745,12 +620,17 @@ def broken(series, index, layout, path): raise PlaneReadError("Page 0 cannot be decoded") return original(series, index, layout, path) - monkeypatch.setattr(planes_module, "_read_page", broken) - result = run_experiment(experiment, ZYX) + with monkeypatch.context() as patch: + patch.setattr(planes_module, "_read_page", broken) + result = run_experiment(experiment, ZYX) found = issue(result.issues, "unreadable_planes") assert found.paths == ("control/bio_rep1/tech_rep1/img_02.tif",) assert "exclude" in found.action - assert not result.written and only_the_report(result.output_folder) + assert not result.written and only_the_report(result.output_folder, staged=True) + # The images that could be read were staged: the next run resumes them (PP-12). + again = run_experiment(experiment, ZYX) + assert again.written and again.read == ("control/bio_rep1/tech_rep1/img_02.tif",) + assert len(again.resumed) == len(IMAGES) - 1 def test_missing_axes_need_confirmation(tmp_path): diff --git a/tools/code_rules_baseline.json b/tools/code_rules_baseline.json index 17ea9845..5257a646 100644 --- a/tools/code_rules_baseline.json +++ b/tools/code_rules_baseline.json @@ -41,7 +41,7 @@ "tests/test_metadata.py: # lines (max #)": 629, "tests/test_notebook_ui.py: # lines (max #)": 1529, "tests/test_planes.py: # lines (max #)": 818, - "tests/test_run.py: # lines (max #)": 982, + "tests/test_run.py: # lines (max #)": 862, "tests/test_zarr_input.py: # lines (max #)": 852, "tests/zarr_tree.py write_ngff: # parameters (max #)": 12 }