From c9f31515a65724f831c0879b5696a2416a5ff923 Mon Sep 17 00:00:00 2001 From: Stephen Aylward Date: Fri, 22 May 2026 15:40:35 -0400 Subject: [PATCH 1/3] ENH: Upaded AI agents for consistency and make physicsnemo optional --- .agents/agents/architecture.md | 13 +- .agents/agents/implementation.md | 22 ++- .agents/agents/testing.md | 75 +++++--- .agents/skills/check-conventions/SKILL.md | 76 +++++++++ .agents/skills/regen-api-map/SKILL.md | 36 ++++ .agents/skills/review-pr/SKILL.md | 81 +++++++++ .github/workflows/README.md | 21 +-- .github/workflows/ci.yml | 27 +-- .github/workflows/nightly-health.yml | 6 +- .github/workflows/test-slow.yml | 10 +- .gitignore | 3 + AGENTS.md | 25 +++ CLAUDE.md | 20 ++- README.md | 13 +- docs/API_MAP.md | 159 +++++++++++------ docs/api/registration/time_series.rst | 4 +- docs/contributing.rst | 5 +- docs/developer/core.rst | 7 +- docs/quickstart.rst | 5 +- docs/testing.rst | 17 +- docs/tutorials.rst | 4 + pyproject.toml | 20 ++- .../register_time_series_images.py | 161 ++++++++++++++---- tests/README.md | 13 +- tests/conftest.py | 79 +++++++-- tests/test_register_time_series_images.py | 24 +++ tutorials/README.md | 4 +- ...utorial_09_physicsnemo_mesh_stage_model.py | 19 ++- utils/ai_agent_github_reviews.py | 6 +- 29 files changed, 760 insertions(+), 195 deletions(-) create mode 100644 .agents/skills/check-conventions/SKILL.md create mode 100644 .agents/skills/regen-api-map/SKILL.md create mode 100644 .agents/skills/review-pr/SKILL.md diff --git a/.agents/agents/architecture.md b/.agents/agents/architecture.md index 41efa5e..1411561 100644 --- a/.agents/agents/architecture.md +++ b/.agents/agents/architecture.md @@ -27,11 +27,18 @@ Use `docs/API_MAP.md` to locate classes and signatures without manual searching. ## Design invariants to preserve -- `PhysioMotion4DBase` inheritance for all major classes. +- `PhysioMotion4DBase` inheritance for runtime workflow / segmentation / + registration / USD classes. Helper, data-container, and standalone-script + classes do not inherit. - Segmenters return anatomy group masks with consistent label IDs. - Image registerers follow: `set_fixed_image()` → `register(moving)` → dict with transforms. - ITK for images; PyVista for surfaces. Boundary is at contour extraction. -- Coordinate system: RAS internally; Y-up only at USD export. +- Coordinate system: LPS internally throughout (images and surfaces). Convert + to USD right-handed Y-up only at USD export via + `vtk_to_usd.lps_points_to_usd`. +- Public entry point for VTK→USD is `ConvertVTKToUSD`. The `vtk_to_usd/` + subpackage is internal; experiments, CLIs, tests, and tutorials must not + import from it directly. ## Output format — always produce all six sections @@ -42,4 +49,4 @@ Use `docs/API_MAP.md` to locate classes and signatures without manual searching. 5. **Open questions** — decisions that need user input before coding starts. 6. **Recommended next action** — one sentence. -Flag any change at the ITK↔PyVista boundary or the RAS→Y-up transform as **high-risk**. +Flag any change at the ITK↔PyVista boundary or the LPS→USD-Y-up transform as **high-risk**. diff --git a/.agents/agents/implementation.md b/.agents/agents/implementation.md index 137906d..2465c75 100644 --- a/.agents/agents/implementation.md +++ b/.agents/agents/implementation.md @@ -26,18 +26,32 @@ Key modules: `physiomotion4d_base.py`, `segment_chest_*.py`, `register_images_*. ## Code rules -- All classes inherit from `PhysioMotion4DBase`. New classes must too. -- Use `self.log_info()` / `self.log_debug()` — never `print()`. +- Runtime workflow / segmentation / registration / USD classes inherit from + `PhysioMotion4DBase`. Standalone scripts, data containers, and helper + classes do not. +- In `PhysioMotion4DBase` subclasses use `self.log_info()` / `self.log_debug()`, + never `print()`. Standalone scripts may use `print()`. +- No emojis in `.py` files. Windows cp1252 has bitten this project; keep + emojis out of code and minimize them in docs. +- Experiments, CLIs, tests, and tutorials must use `ConvertVTKToUSD`. Never + import directly from `vtk_to_usd` outside of `convert_vtk_to_usd.py` and + the `vtk_to_usd/` subpackage itself. +- Scripts that instantiate `SegmentChestTotalSegmentator` must guard the + top-level invocation with `if __name__ == "__main__":` on Windows + (`torch.multiprocessing` requires it). - Single quotes for strings; double quotes for docstrings. 88-char line limit. - Full type hints; `Optional[X]` not `X | None` (mypy UP007 is suppressed). - `pathlib.Path` for all file paths. `subprocess.run(check=True, text=True)` — no `os.system`. -- Run `ruff check . --fix && ruff format .` after every Python edit. +- After every Python edit run `python -m ruff check . --fix && python -m ruff format .` + from the active `.\venv`. ## Data shapes — state them explicitly - ITK images: axes X, Y, Z [, T] in LPS world space (ITK's native frame). - 4D time series: shape `(X, Y, Z, T)`. Never silently squeeze or permute. -- PyVista surfaces: RAS internally; Y-up only at USD export. +- PyVista surfaces: LPS internally (inherited from `itk.vtk_image_from_image`). + Convert to USD right-handed Y-up only at USD export, via + `vtk_to_usd.lps_points_to_usd` (USD +X=Left, +Y=Superior, +Z=Anterior). - Name shape variables explicitly: `n_frames`, `spatial_shape`, not bare integer indices. ## What not to do diff --git a/.agents/agents/testing.md b/.agents/agents/testing.md index 0fd5fcb..38158a7 100644 --- a/.agents/agents/testing.md +++ b/.agents/agents/testing.md @@ -1,45 +1,68 @@ --- name: PhysioMotion4D Testing Agent -description: Writes and updates pytest tests for PhysioMotion4D. Prefers synthetic itk.Image and PyVista surfaces over real data, states tensor shapes explicitly, and uses baseline utilities for regression. +description: Writes and updates pytest tests for PhysioMotion4D. Strongly prefers real downloaded data via session fixtures, states tensor shapes explicitly, and uses baseline utilities for regression. tools: Read, Edit, Write, Bash, Glob, Grep --- -You are a testing agent for PhysioMotion4D. Write correct, fast, synthetic-data-driven -pytest tests that exercise the library's scientific pipelines. +You are a testing agent for PhysioMotion4D. Write correct pytest tests that +exercise the library's scientific pipelines using real downloaded data +wherever practical. ## Test architecture - `tests/conftest.py` — session-scoped fixtures chaining: download → convert → segment → register - `tests/baselines/` — stored via Git LFS; fetch with `git lfs pull` -- `src/physiomotion4d/test_tools.py` — baseline comparison utilities +- `src/physiomotion4d/test_tools.py` — baseline comparison utilities (`TestTools`) - Markers (all opt-in via `--run-`): `slow`, `requires_gpu`, - `requires_simpleware`, `experiment`, `tutorial`. Tests that need - downloadable data fetch it through the session fixtures and run by default. - -## Run commands (use `py`, not `python`) - -```bash -py -m pytest tests/ -v # fast, recommended (slow/GPU/etc auto-skipped) -py -m pytest tests/test_contour_tools.py -v # single file -py -m pytest tests/test_contour_tools.py::TestContourTools -v # single class -py -m pytest tests/ -v --run-slow # opt into slow tests -py -m pytest tests/ -v --run-gpu --run-slow # typical local GPU profile (CI runner adds --run-simpleware --run-experiments --run-tutorials) -py -m pytest tests/ --create-baselines # create missing baselines + `requires_simpleware`, `experiment`, `tutorial`. The `requires_data` marker + no longer exists — tests that need downloadable data pull it through the + session fixtures and run by default. + +## Run commands + +Activate `.\venv` first (`.\venv\Scripts\Activate.ps1`); `python` then resolves +to the project interpreter. If activation is impossible, use +`.\venv\Scripts\python.exe -m ...` directly. + +```powershell +python -m pytest tests/ -v # fast, recommended (slow/GPU/etc auto-skipped) +python -m pytest tests/test_contour_tools.py -v # single file +python -m pytest tests/test_contour_tools.py::TestContourTools -v # single class +python -m pytest tests/ -v --run-slow # opt into slow tests +python -m pytest tests/ -v --run-gpu --run-slow # typical local GPU profile (CI runner adds --run-simpleware --run-experiments --run-tutorials) +python -m pytest tests/ --create-baselines # create missing baselines ``` ## Writing tests — rules 1. Read the implementation file first; understand the public interface. -2. Propose a test plan: what behaviors to cover, what synthetic data to create. -3. Build synthetic `itk.Image` objects or small `pv.PolyData` surfaces — 32–64 voxels/side. - When real data is unavoidable, request the standard fixtures - (`test_directories`, `download_test_data`, `test_images`) — the data is - downloaded automatically on first use. -4. State image shape and axis order in the test docstring: - e.g. `"""...image shape: (64, 64, 32), axes: X, Y, Z."""` -5. Use `test_tools.py` baseline utilities for surface and image regression checks. -6. One logical assertion per test where possible. -7. Do not mock segmentation or registration models — test real outputs on synthetic data. +2. Propose a test plan: what behaviors to cover, what inputs each needs. +3. **Strongly prefer real downloaded test data over synthetic.** Request the + session fixtures `test_directories`, `download_test_data`, and + `test_images` so the standard datasets are fetched automatically on first + use. Real data exercises preprocessing, resampling, dtype handling, and + world-frame metadata paths that synthetic toy volumes silently bypass. +4. Only fall back to synthetic `itk.Image` or `pv.PolyData` inputs when: + - the behavior under test is a pure unit (axis arithmetic, dict routing, + etc.) where real data adds no signal, or + - real data would push the test into a slow / GPU / Simpleware bucket + that does not fit the test's purpose. + When synthetic is unavoidable, keep volumes ≤64 voxels per side and say so + in the docstring. +5. State image shape and axis order in every test docstring, e.g. + `"""...image shape: (X, Y, Z, T) = (64, 64, 32, 1), LPS world frame."""`. +6. When a test produces an image or surface, compare against a baseline using + `test_tools.py` utilities (`TestTools`) rather than ad-hoc value asserts. + Store baselines under `tests/baselines/` (Git LFS-tracked). +7. Prefer images from `ROOT/data/test/slicer_heart_small`. +8. Prefer storing results in subdirectories under `./results/`. +9. Mark tests that need a GPU, slow runtime, or licensed Simpleware install + with `@pytest.mark.requires_gpu`, `@pytest.mark.slow`, or + `@pytest.mark.requires_simpleware`. Mark experiment and tutorial tests + with `@pytest.mark.experiment` or `@pytest.mark.tutorial`. Tests that just + need downloadable data need no marker. +10. Do not mock segmentation or registration models — test real outputs. +11. No emojis in test files (Windows cp1252 encoding has bitten this project). ## Naming diff --git a/.agents/skills/check-conventions/SKILL.md b/.agents/skills/check-conventions/SKILL.md new file mode 100644 index 0000000..dc1823f --- /dev/null +++ b/.agents/skills/check-conventions/SKILL.md @@ -0,0 +1,76 @@ +--- +description: Audit changed files (or a given path) against PhysioMotion4D's hard project rules — base-class inheritance, logging, coordinate conventions, USD entry point, Windows multiprocessing guard, quoting, type-hint style, line length, and emoji ban. Reports violations without auto-fixing. +--- + +Audit PhysioMotion4D source for hard-rule violations. + +$ARGUMENTS + +By default, audit every Python file modified since `HEAD`. If $ARGUMENTS names +specific files or directories, audit those instead. + +## Determine the file set + +```powershell +# Default: changed, non-deleted .py files since HEAD +git diff --diff-filter=d --name-only HEAD -- '*.py' +``` + +If a path was passed in $ARGUMENTS, expand it to all `.py` files under that +path (recursively). Skip files that no longer exist on disk. + +## Rules to check + +For each file, read the **entire file** (rules below depend on surrounding +context such as class inheritance), then flag every occurrence of: + +### Base class and logging +- [ ] A class that orchestrates workflow / segmentation / registration / USD + conversion but does **not** inherit from `PhysioMotion4DBase`. +- [ ] A `print(` call inside the body of a class that inherits from + `PhysioMotion4DBase` (it must use `self.log_info()` / `self.log_debug()`). + Standalone scripts and helper / data-container classes may use `print()`. + +### USD / coordinate conventions +- [ ] An `import` of `physiomotion4d.vtk_to_usd` (or `from ... vtk_to_usd ...`) + from a file that is **not** `src/physiomotion4d/convert_vtk_to_usd.py` + and is **not** itself inside `src/physiomotion4d/vtk_to_usd/`. + Experiments, CLIs, tests, and tutorials must use `ConvertVTKToUSD`. +- [ ] A docstring or comment claiming PyVista surfaces are in **RAS** — they + are in **LPS** internally; convert to USD Y-up only at export. + +### Windows multiprocessing +- [ ] A module-level instantiation of `SegmentChestTotalSegmentator` (or a + module-level call into it) that is not guarded by + `if __name__ == "__main__":`. Required on Windows because + `torch.multiprocessing` re-imports the module in child workers. + +### Code style +- [ ] `X | None` in a type hint (use `Optional[X]`; ruff `UP007` is suppressed). +- [ ] `Any` in a public signature without a comment explaining why. +- [ ] A docstring delimited with `'''` (use `"""`). +- [ ] A string literal using `"..."` for ordinary inline strings (use `'...'`; + docstrings stay on `"""`). +- [ ] A line longer than 88 characters. +- [ ] An emoji or other non-ASCII glyph inside a `.py` file (Windows cp1252 + encoding has broken builds; keep emojis out of source). + +### Public API hygiene +- [ ] A public method (no leading underscore) without a NumPy-style docstring. +- [ ] An array / image parameter or return value whose docstring does not + state shape and axis order. + +## Output + +Group findings by file. For each finding, print: + +``` +: +``` + +End with a one-line summary: total findings per rule category. + +Do **not** auto-fix. The point is to surface violations the user can decide +how to address. If `$ARGUMENTS` includes `--fix`, ask before mutating anything +and limit fixes to the trivially mechanical rules (line length is not one of +them — `ruff format` covers that). diff --git a/.agents/skills/regen-api-map/SKILL.md b/.agents/skills/regen-api-map/SKILL.md new file mode 100644 index 0000000..d9baf89 --- /dev/null +++ b/.agents/skills/regen-api-map/SKILL.md @@ -0,0 +1,36 @@ +--- +description: Regenerate docs/API_MAP.md from the current source tree and report whether the public API surface changed. Run after editing any public class, method, or function signature in src/physiomotion4d. +--- + +Regenerate the PhysioMotion4D API map and report what changed. + +$ARGUMENTS + +Instructions: + +1. Run the generator from the active `.\venv` (activate it first if needed, + or call `.\venv\Scripts\python.exe` directly): + + ```powershell + python utils/generate_api_map.py + ``` + +2. Inspect the diff: + + ```powershell + git diff -- docs/API_MAP.md + ``` + +3. Report one of: + - **No change** — public API surface is identical; nothing to commit. + - **Changed** — list each added, removed, or signature-modified entry + (class.method, file, one-line summary). Group by file. + +4. If `docs/API_MAP.md` changed, remind the user to stage it together with + the source changes that produced it so the API map never lags the code. + +5. Do not edit `docs/API_MAP.md` by hand. If the generator output looks wrong, + investigate `utils/generate_api_map.py` and the source it scans, not the + generated file. + +6. Do not commit. The user controls when to stage and commit. diff --git a/.agents/skills/review-pr/SKILL.md b/.agents/skills/review-pr/SKILL.md new file mode 100644 index 0000000..54fcdd3 --- /dev/null +++ b/.agents/skills/review-pr/SKILL.md @@ -0,0 +1,81 @@ +--- +description: Screen a GitHub PR's review comments by invoking utils/ai_agent_github_reviews.py. Fetches inline threads and PR-level reviews via gh CLI, applies accepted edits as pending working-tree changes, resolves processed threads, and writes a Markdown summary tagged by PR number. +--- + +Process review comments on a GitHub PR using the project's PR-review driver. + +$ARGUMENTS + +## Usage shape + +`$ARGUMENTS` is typically just a PR number, optionally followed by flags +forwarded to `utils/ai_agent_github_reviews.py`. Examples: + +- `/review-pr 42` +- `/review-pr 42 --agent claude` +- `/review-pr 42 --since-last-push` +- `/review-pr 42 --prompt-only` +- `/review-pr 42 --no-resolve` + +If `$ARGUMENTS` is empty, ask the user for the PR number before doing anything. + +## Preconditions + +1. Confirm `gh` is installed and authenticated: + ```powershell + gh auth status + ``` + If not authenticated, stop and tell the user to run `gh auth login`. + +2. Confirm the current branch is **not** `main` and not detached. If it is, + warn the user — applied edits will land on whichever branch is checked + out. Ask before proceeding. + +3. Confirm the working tree has no unstaged Python edits the user would not + want mixed with applied review suggestions: + ```powershell + git status --short + ``` + If there are pending changes, surface them and ask whether to continue. + +## Run + +Invoke the driver from the active `.\venv` (activate first if needed): + +```powershell +python utils/ai_agent_github_reviews.py --pr [flags...] +``` + +Defaults worth knowing: +- `--agent codex` is the script default. Pass `--agent claude` to drive + Claude Code instead. +- Without `--no-resolve`, every processed inline-comment thread is marked + resolved on GitHub after the agent finishes. +- With `--since-last-push`, only comments posted after this clone last saw + the remote PR head branch move are included. +- `--prompt-only` prints the prompt without invoking an agent or resolving + anything. + +## After the run + +1. Read `pr__review_summary.md` written to the repo root. Report: + - Total comments processed. + - APPLY / REVISE / REJECT counts. + - Any rejection that cited a hard rule from AGENTS.md. + +2. Show the resulting working-tree diff so the user can decide what to keep: + ```powershell + git diff --stat + git diff + ``` + +3. Do **not** stage or commit. The script applies edits as pending + working-tree changes only; the user controls staging + (`git add -p`) and commit. + +4. If the run failed: + - "gh CLI not found" → tell the user to install via + `winget install GitHub.cli` then `gh auth login`. + - "claude CLI not found" → tell the user to install via + `winget install Anthropic.ClaudeCode`, or retry with `--agent codex`. + - Any other error: surface the script's stderr verbatim and stop. diff --git a/.github/workflows/README.md b/.github/workflows/README.md index dd77756..9f3fb5b 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -144,12 +144,12 @@ GPU tests require self-hosted runners with: **Option 3: Run Locally** ```bash -# Install with CUDA support -uv pip install -e ".[test,cuda13]" +# Install with CUDA + PhysicsNeMo (matches the self-hosted GPU runner) +uv pip install -e ".[test,cuda13,physicsnemo]" # Run GPU tests -pytest tests/ -v -m "not slow" -pytest tests/ -v -m "slow" # For long-running tests +pytest tests/ -v --run-gpu +pytest tests/ -v --run-all # Match CI: enable every --run-* bucket ``` ### GitHub-Hosted Runners @@ -217,14 +217,15 @@ pytest tests/ -m "unit and not requires_gpu" --cov=physiomotion4d ### GPU Tests ```bash -# Install with CUDA support -uv pip install -e ".[test,cuda13]" +# Install with CUDA + PhysicsNeMo (matches the self-hosted GPU runner) +uv pip install -e ".[test,cuda13,physicsnemo]" -# Run all tests (including GPU) -pytest tests/ -m "not slow" +# Run GPU tests +pytest tests/ --run-gpu -# Run slow tests -pytest tests/ -m "slow" +# Enable every --run-* bucket at once (slow, GPU, simpleware, +# physicsnemo, experiments, tutorials) +pytest tests/ --run-all ``` ## Coverage Reports diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1c79fa3..3ccb7f6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -9,11 +9,13 @@ name: CI # - Code quality: Linting and formatting checks # # Test markers (gated by opt-in pytest flags; default = skip): -# - slow -> --run-slow -# - requires_gpu -> --run-gpu -# - requires_simpleware -> --run-simpleware (also implies GPU) -# - experiment -> --run-experiments -# - tutorial -> --run-tutorials +# - slow -> --run-slow +# - requires_gpu -> --run-gpu +# - requires_simpleware -> --run-simpleware (also implies GPU) +# - requires_physicsnemo -> --run-physicsnemo (needs the [physicsnemo] extra) +# - experiment -> --run-experiments +# - tutorial -> --run-tutorials +# --run-all enables every bucket above at once. # Tests that need external data download it automatically via fixtures. on: @@ -300,10 +302,12 @@ jobs: python -m pip install --upgrade pip setuptools wheel uv - name: Install package with test dependencies - # uv respects [tool.uv.sources], routing torch to pytorch-cu130 for cuda13 + # uv respects [tool.uv.sources], routing torch to pytorch-cu130 for cuda13. + # The [physicsnemo] extra brings in nvidia-physicsnemo so Tutorial 9 and + # requires_physicsnemo-marked tests can run on this GPU runner. # Invoke via python -m uv so uv targets the active venv interpreter. run: | - python -m uv pip install -e ".[test,cuda13]" + python -m uv pip install -e ".[test,cuda13,physicsnemo]" - name: Assert CUDA is accessible run: | @@ -329,12 +333,12 @@ jobs: pip list - name: Run GPU tests - # External self-hosted GPU runner: enable every opt-in bucket. + # External self-hosted GPU runner: enable every opt-in bucket via --run-all. # Tests whose host requirements (e.g. a licensed Simpleware install) # aren't met on the runner will runtime-skip cleanly via their # internal availability guards. run: | - pytest tests/ -v --run-gpu --run-slow --run-simpleware --run-experiments --run-tutorials --cov=physiomotion4d --cov-report=xml --cov-report=term --cov-report=html + pytest tests/ -v --run-all --cov=physiomotion4d --cov-report=xml --cov-report=term --cov-report=html env: CUDA_VISIBLE_DEVICES: 0 @@ -417,8 +421,9 @@ jobs: # pytest tests/ -v --run-simpleware --run-gpu --run-slow # Full Simpleware coverage # pytest tests/test_register_images_ants.py -v --run-slow # -# Self-hosted GPU runner enables ALL buckets: -# --run-gpu --run-slow --run-simpleware --run-experiments --run-tutorials +# Self-hosted GPU runner enables ALL buckets via --run-all +# (--run-gpu --run-slow --run-simpleware --run-physicsnemo --run-experiments --run-tutorials). +# That runner installs the [physicsnemo] extra in addition to [test,cuda13]. # # To run experiment tests (manual only, extremely slow): # pytest tests/test_experiments.py -v --run-experiments diff --git a/.github/workflows/nightly-health.yml b/.github/workflows/nightly-health.yml index f2fb3e2..53e31c0 100644 --- a/.github/workflows/nightly-health.yml +++ b/.github/workflows/nightly-health.yml @@ -79,9 +79,11 @@ jobs: - name: Install uv and package # Invoke via python -m uv so uv targets the active venv interpreter. + # The [physicsnemo] extra is required for --run-physicsnemo (Tutorial 9 + # and any tests marked requires_physicsnemo). run: | python -m pip install --upgrade pip uv - python -m uv pip install -e ".[test,cuda13]" + python -m uv pip install -e ".[test,cuda13,physicsnemo]" - name: Assert CUDA is accessible run: | @@ -108,7 +110,7 @@ jobs: # The step outcome (success/failure) is still captured and passed downstream. continue-on-error: true run: | - pytest tests/ -v --run-experiments --run-tutorials --run-slow --run-gpu --run-simpleware ` + pytest tests/ -v --run-all ` --cov=physiomotion4d ` --cov-report=xml ` --cov-report=json ` diff --git a/.github/workflows/test-slow.yml b/.github/workflows/test-slow.yml index 7e1b2b4..be9035e 100644 --- a/.github/workflows/test-slow.yml +++ b/.github/workflows/test-slow.yml @@ -51,10 +51,12 @@ jobs: python -m pip install --upgrade pip setuptools wheel uv - name: Install package with test dependencies - # uv respects [tool.uv.sources], routing torch to pytorch-cu130 for cuda13 + # uv respects [tool.uv.sources], routing torch to pytorch-cu130 for cuda13. + # The [physicsnemo] extra is required for --run-physicsnemo (Tutorial 9 + # and any tests marked requires_physicsnemo). # Invoke via python -m uv so uv targets the active venv interpreter. run: | - python -m uv pip install -e ".[test,cuda13]" + python -m uv pip install -e ".[test,cuda13,physicsnemo]" - name: Assert CUDA is accessible run: | @@ -72,12 +74,12 @@ jobs: " - name: Run slow tests - # Self-hosted GPU runner: enable every opt-in bucket. + # Self-hosted GPU runner: enable every opt-in bucket via --run-all. # Tests whose host requirements (e.g. a licensed Simpleware install) # aren't met on the runner will runtime-skip cleanly via their # internal availability guards. run: | - pytest tests/ -v --run-slow --run-gpu --run-simpleware --run-experiments --run-tutorials --cov=physiomotion4d --cov-report=xml --cov-report=term + pytest tests/ -v --run-all --cov=physiomotion4d --cov-report=xml --cov-report=term env: CUDA_VISIBLE_DEVICES: 0 diff --git a/.gitignore b/.gitignore index 903efe9..9af39e1 100644 --- a/.gitignore +++ b/.gitignore @@ -14,6 +14,9 @@ __pycache__ _version.py htmlcov +# AI files +.claude + # Documentation build files docs/_build/ docs/_static/.buildinfo diff --git a/AGENTS.md b/AGENTS.md index 293e4dc..745ce10 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -76,9 +76,13 @@ python -m pytest tests/test_contour_tools.py::test_extract_surface -v python -m pytest tests/ -v --run-slow python -m pytest tests/ -v --run-gpu python -m pytest tests/ -v --run-simpleware +python -m pytest tests/ -v --run-physicsnemo python -m pytest tests/ -v --run-experiments python -m pytest tests/ -v --run-tutorials +# Enable every bucket at once (equivalent to passing all --run-* flags) +python -m pytest tests/ -v --run-all + # Typical local GPU profile python -m pytest tests/ -v --run-gpu --run-slow @@ -91,6 +95,18 @@ python -m pytest tests/ --create-baselines Version bumping: `bumpver update --patch`, `--minor`, or `--major`. +## Codex Sandbox + +- If a Python command fails with + `No Python at '"C:\Users\saylward\AppData\Local\Programs\Python\Python311\python.exe'`, + do not assume Python 3.11 is missing. The Codex sandbox can break the + launcher or venv path. +- Use the temporary validation environment instead: + `C:\Users\saylward\AppData\Local\Temp\pm4d-uv-env\Scripts\python.exe`. + It has been verified as Python 3.11.9 with `ruff` available. +- Run that temporary venv outside the sandbox when needed. Treat this as an + environment/sandbox workaround, not a dependency or installation problem. + ## Universal Rules - Read the relevant source files before proposing changes. @@ -99,6 +115,15 @@ Version bumping: `bumpver update --patch`, `--minor`, or `--major`. utility scripts and data/container/helper classes do not. - In classes that inherit from `PhysioMotion4DBase`, use `self.log_info()` and `self.log_debug()`, never `print()`. Standalone scripts may use `print()`. +- No emojis in `.py` files; avoid them in docs too. Windows cp1252 encoding + has broken this project before. +- The public VTK→USD entry point is `ConvertVTKToUSD`. Experiments, CLIs, + tests, and tutorials must use it. Do not import from the `vtk_to_usd/` + subpackage directly outside of `convert_vtk_to_usd.py` and the subpackage + itself. +- Scripts that instantiate `SegmentChestTotalSegmentator` must guard the + top-level invocation with `if __name__ == "__main__":` on Windows + (`torch.multiprocessing` requires it). - Single quotes for strings; double quotes for docstrings. Keep lines at or below 88 characters. - Full type hints are required under strict mypy. Use `Optional[X]`, not diff --git a/CLAUDE.md b/CLAUDE.md index b33affa..bf268f0 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -50,11 +50,15 @@ py -m pytest tests/test_contour_tools.py::test_extract_surface -v py -m pytest tests/ -v --run-slow # tests marked 'slow' py -m pytest tests/ -v --run-gpu # tests marked 'requires_gpu' py -m pytest tests/ -v --run-simpleware # tests marked 'requires_simpleware' +py -m pytest tests/ -v --run-physicsnemo # tests marked 'requires_physicsnemo' py -m pytest tests/ -v --run-experiments # tests marked 'experiment' py -m pytest tests/ -v --run-tutorials # tests marked 'tutorial' -# Typical local GPU profile. The self-hosted CI GPU runner enables every -# bucket: --run-gpu --run-slow --run-simpleware --run-experiments --run-tutorials +# Enable every bucket at once (equivalent to passing all --run-* flags). +# Self-hosted CI GPU runner uses this after installing [test,cuda13,physicsnemo]. +py -m pytest tests/ -v --run-all + +# Typical local GPU profile. py -m pytest tests/ -v --run-gpu --run-slow # With coverage @@ -122,8 +126,16 @@ Claude, Codex, and other AI tooling. - `/plan` — inspect files, summarize design, produce a numbered plan (no code changes) - `/impl` — read → summarize → plan → implement in small diffs -- `/test-feature` — propose test plan, write synthetic-data pytest tests -- `/doc-feature` — update docstrings and regenerate API map +- `/test-feature` — propose test plan, write real-data-driven pytest tests with baselines +- `/doc-feature` — update docstrings (and remind you to run `/regen-api-map`) +- `/regen-api-map` — regenerate `docs/API_MAP.md` and report public-API changes +- `/check-conventions` — audit changed files against project hard rules + (base-class, logging, coordinate frame, USD entry point, Windows mp guard, + quoting, type hints, line length, emoji ban) +- `/simplify-staged` — readability / quality pass over `git diff HEAD` +- `/commit` — stage tracked changes, draft `: …` message, loop until hooks pass +- `/review-pr ` — drive `utils/ai_agent_github_reviews.py` to triage + a PR's review comments and apply accepted edits as pending changes ## File Operations diff --git a/README.md b/README.md index 3a1ff5e..79b08fb 100644 --- a/README.md +++ b/README.md @@ -69,6 +69,17 @@ The `[cuda13]` extra installs CuPy. In uv-managed source environments, PyTorch, torchvision, and torchaudio resolve from the CUDA 13.0 PyTorch wheel index. There is no need to install PyTorch separately. +PhysicsNeMo (used only by Tutorial 9) is an optional extra because it brings in +a large CUDA-only toolchain and narrows supported Python versions. Install it +explicitly when needed: + +```bash +pip install "physiomotion4d[physicsnemo]" +``` + +PhysicsNeMo itself requires Python >= 3.11. PhysioMotion4D otherwise supports +Python >= 3.10. + ### Installation from Source 1. **Clone the repository** (Git LFS is required for tests; install it first from [git-lfs.github.com](https://git-lfs.github.com)): @@ -169,7 +180,7 @@ major workflow. They are the recommended starting point for new users. | 6 | `tutorials/tutorial_06_reconstruct_highres_4d_ct.py` | Reconstruct high-res 4D CT | DirLab-4DCT (manual) | | 7 | `tutorials/tutorial_07_dirlab_pca_model.py` | Build a surface PCA lung-lobe model and fit all cases | DirLab-4DCT (manual) | | 8 | `tutorials/tutorial_08_dirlab_pca_time_series.py` | Propagate PCA-fitted lung-lobe meshes through DirLab time series | DirLab-4DCT plus Tutorial 7 output | -| 9 | `tutorials/tutorial_09_physicsnemo_mesh_stage_model.py` | Train a PhysicsNeMo mesh stage model | Tutorial 8 output | +| 9 | `tutorials/tutorial_09_physicsnemo_mesh_stage_model.py` | Train a PhysicsNeMo mesh stage model (requires `[physicsnemo]` extra; Python >= 3.11) | Tutorial 8 output | Each tutorial is a `# %%` percent-cell Python script. Paths are defined near the top of the script; edit those constants for custom data/output locations, diff --git a/docs/API_MAP.md b/docs/API_MAP.md index dec4930..f1f04e3 100644 --- a/docs/API_MAP.md +++ b/docs/API_MAP.md @@ -28,21 +28,66 @@ _Re-run `py utils/generate_api_map.py` whenever public APIs change._ ## experiments/LongitudinalRegistration/0-cardiacGatedCT_segment_and_landmark.py -- `def segment_images(src_data_dirs, src_data_files)` (line 57): Segment each image with SegmentHeartSimpleware and save labelmaps. +- `def segment_images(src_data_dirs, src_data_files)` (line 63): Segment each image with SegmentHeartSimpleware and save labelmaps. ## experiments/LongitudinalRegistration/1-finetune_icon.py -- `def get_segmented_images(src_data_dirs, src_data_files)` (line 84): Segment each image with SegmentHeartSimpleware and save labelmaps. +- `def get_segmented_images(src_data_dirs, src_data_files)` (line 81): Segment each image with SegmentHeartSimpleware and save labelmaps. - `def get_mask_images(src_data_dirs, src_data_files)` (line 117): Get mask images for each image. -## experiments/LongitudinalRegistration/recon_4d_icon_finetuned.py - -- `def convert_labelmap_to_masks(labelmap_file, output_dir)` (line 82) -- `def register_time_series(reference_image_file, reference_labelmap_file, source_image_dir, source_image_files, segmented_image_files, weights_path)` (line 91) +## experiments/LongitudinalRegistration/2-run_registration_comparison.py + +- **class MethodSpec** (line 49): Registration method plus optional ICON checkpoint. +- **class ImageArtifacts** (line 58): Input files associated with one image volume. +- `def nii_stem(path)` (line 67): Return a stable stem for ``.nii.gz`` or single-suffix files. +- `def timepoint_from_name(path)` (line 74): Extract the gated time-point tag from a filename. +- `def first_existing(paths)` (line 82): Return the first existing path from a candidate list. +- `def landmark_candidates(image_file, segmentation_dir, artifact_dir)` (line 90): Return likely landmark CSV paths for an image. +- `def labelmap_candidates(image_file, segmentation_dir, artifact_dir)` (line 114): Return likely labelmap paths for an image. +- `def image_artifacts(image_file, segmentation_dir, artifact_dir=None)` (line 132): Find landmarks and labelmaps associated with one image. +- `def read_landmarks(path)` (line 150): Read physical LPS landmarks from ``Name,X,Y,Z`` CSV. +- `def write_landmarks(path, landmarks)` (line 163): Write physical LPS landmarks to ``Name,X,Y,Z`` CSV. +- `def transform_landmarks(landmarks, transform)` (line 173): Apply an ITK physical-space transform to landmark coordinates. +- `def landmark_errors(source, target)` (line 186): Return per-landmark Euclidean errors in millimeters. +- `def summarize_errors(errors, prefix)` (line 196): Summarize landmark errors for one comparison mode. +- `def write_error_details(path, subject_id, method_name, timepoint, mode, errors)` (line 214): Append per-landmark errors to the detail CSV. +- `def dice_by_label(labelmap_a, labelmap_b)` (line 243): Compute Dice scores for labels present in either 3D labelmap. +- `def summarize_dice(scores)` (line 265): Summarize per-label Dice scores. +- `def discover_subjects(reference_dir, timepoint_base_dir, reference_pattern, timepoint_pattern, exclude_tokens, segmentation_dir, segmentation_base_dir)` (line 277): Discover reference and time-point files for each subject. +- `def build_method_specs(method_names, finetuned_weights_path)` (line 329): Map output method labels to registrar methods and optional weights. +- `def configure_registrar(method_spec, fixed_image, fixed_labelmap, ants_iterations, greedy_iterations, icon_iterations)` (line 361): Create and configure the time-series registrar. +- `def write_summary(path, rows)` (line 384): Write experiment summary rows. +- `def run_method_for_subject(subject_id, reference_artifacts, timepoint_artifacts, method_spec, output_dir, run_resegmentation, ants_iterations, greedy_iterations, icon_iterations, error_detail_file)` (line 396): Run one registration method for one subject and return summary rows. +- `def parse_iterations(value)` (line 561): Parse comma-separated multi-resolution iteration counts. +- `def main()` (line 566): Run the longitudinal registration comparison experiment. + +## experiments/LongitudinalRegistration/recon_4d_icon_eval.py + +- **class MethodSpec** (line 67): Output label and optional ICON checkpoint for one registration run. +- **class TimepointArtifacts** (line 75): File paths for one gated time-point: image, labelmap, landmarks. +- `def nii_stem(path)` (line 84): Return the stem of a ``.nii.gz`` (or single-suffix) file. +- `def timepoint_from_name(path)` (line 91): Extract the gated time-point tag (``g###``) from a filename. +- `def select_reference_index(num_frames, percentile)` (line 99): Return the frame index closest to ``percentile`` of the series. +- `def discover_subject(subject_id, timepoint_base_dir, segmentation_base_dir, exclude_tokens)` (line 108): Discover gated images plus their labelmap and landmark companions. +- `def discover_subject_ids(timepoint_base_dir, segmentation_base_dir)` (line 149): Return subject IDs that have both gated and segmentation directories. +- `def read_landmarks(path)` (line 166): Read physical LPS landmarks from a ``Name,X,Y,Z`` CSV file. +- `def write_landmarks(path, landmarks)` (line 179): Write physical LPS landmarks to a ``Name,X,Y,Z`` CSV file. +- `def transform_landmarks(landmarks, transform)` (line 189): Apply an ITK physical-space transform to landmark coordinates. +- `def landmark_errors(source, target)` (line 202): Return per-landmark Euclidean errors in millimeters. +- `def summarize_errors(errors, prefix)` (line 212): Summarize landmark errors for one comparison mode. +- `def dice_by_label(labelmap_a, labelmap_b)` (line 230): Compute Dice for every non-zero label present in either 3D labelmap. +- `def summarize_dice(scores)` (line 249): Summarize per-label Dice scores into mean and minimum. +- `def write_error_details(path, subject_id, method_name, timepoint, mode, errors)` (line 261): Append per-landmark errors to the long-form detail CSV. +- `def read_error_details(path)` (line 290): Read the long-form per-landmark error CSV. +- `def print_summary_table(detail_file)` (line 299): Print a high-level table comparing methods for each landmark mode. +- `def write_summary(path, rows)` (line 366): Write the wide-form summary CSV. +- `def mask_from_labelmap(labelmap)` (line 378): Return a uint8 binary mask covering the non-zero labels. +- `def run_method_for_subject(subject_id, timepoint_artifacts, reference_index, method_spec, output_dir, icon_iterations, run_resegmentation, error_detail_file)` (line 386): Run one ICON method for one subject and return per-timepoint rows. +- `def main()` (line 535): Run the ICON default-vs-finetuned comparison experiment. ## experiments/LongitudinalRegistration/recon_4d_run.py -- `def register_time_series(reference_image_file, source_image_dir, source_image_files, registration_method)` (line 69) +- `def register_time_series(reference_image_file, source_image_dir, source_image_files, registration_method)` (line 75) ## experiments/LongitudinalRegistration/uniGradICON/scripts/prepare_l2r_datasets.py @@ -598,18 +643,19 @@ _Re-run `py utils/generate_api_map.py` whenever public APIs change._ ## src/physiomotion4d/register_time_series_images.py -- **class RegisterTimeSeriesImages** (line 23): Register a time series of images to a fixed image. - - `def __init__(self, registration_method='ants', log_level=logging.INFO)` (line 77): Initialize the time series image registration class. - - `def set_number_of_iterations_ants(self, number_of_iterations_ants)` (line 110): Set the number of iterations for ANTs registration. - - `def set_number_of_iterations_icon(self, number_of_iterations_icon)` (line 121): Set the number of iterations for ICON registration. - - `def set_smooth_prior_transform_sigma(self, smooth_prior_transform_sigma)` (line 129): Set the sigma for smoothing the prior transform. - - `def set_mask_dilation(self, mask_dilation_mm)` (line 139): Set the dilation of the fixed and moving image masks. - - `def set_modality(self, modality)` (line 149): Set the imaging modality for registration optimization. - - `def set_fixed_image(self, fixed_image)` (line 159): Set the fixed image for registration. - - `def set_fixed_mask(self, fixed_mask)` (line 170): Set a binary mask for the fixed image region of interest. - - `def register_time_series(self, moving_images, moving_masks=None, moving_labelmaps=None, reference_frame=0, register_reference=True, prior_weight=0.0)` (line 180): Register a time series of images to the fixed image. - - `def reconstruct_time_series(self, moving_images, inverse_transforms, upsample_to_fixed_resolution=False)` (line 534): Reconstruct time series images using inverse transforms. - - `def registration_method(self, moving_image, moving_mask=None, moving_labelmap=None, moving_image_pre=None, initial_forward_transform=None)` (line 664): Registration method required by RegisterImagesBase. +- **class RegisterTimeSeriesImages** (line 25): Register a time series of images to a fixed image. + - `def __init__(self, registration_method='ants', log_level=logging.INFO)` (line 80): Initialize the time series image registration class. + - `def set_number_of_iterations_ants(self, number_of_iterations_ants)` (line 117): Set the number of iterations for ANTs registration. + - `def set_number_of_iterations_icon(self, number_of_iterations_icon)` (line 128): Set the number of iterations for ICON registration. + - `def set_number_of_iterations_greedy(self, number_of_iterations_greedy)` (line 136): Set the number of iterations for Greedy registration. + - `def set_smooth_prior_transform_sigma(self, smooth_prior_transform_sigma)` (line 147): Set the sigma for smoothing the prior transform. + - `def set_mask_dilation(self, mask_dilation_mm)` (line 157): Set the dilation of the fixed and moving image masks. + - `def set_modality(self, modality)` (line 167): Set the imaging modality for registration optimization. + - `def set_fixed_image(self, fixed_image)` (line 177): Set the fixed image for registration. + - `def set_fixed_mask(self, fixed_mask)` (line 188): Set a binary mask for the fixed image region of interest. + - `def register_time_series(self, moving_images, moving_masks=None, moving_labelmaps=None, reference_frame=0, register_reference=True, prior_weight=0.0)` (line 198): Register a time series of images to the fixed image. + - `def reconstruct_time_series(self, moving_images, inverse_transforms, upsample_to_fixed_resolution=False)` (line 607): Reconstruct time series images using inverse transforms. + - `def registration_method(self, moving_image, moving_mask=None, moving_labelmap=None, moving_image_pre=None, initial_forward_transform=None)` (line 737): Registration method required by RegisterImagesBase. ## src/physiomotion4d/segment_anatomy_base.py @@ -839,23 +885,23 @@ _Re-run `py utils/generate_api_map.py` whenever public APIs change._ ## tests/conftest.py -- `def pytest_addoption(parser)` (line 35): Add custom command-line options for pytest. -- `def pytest_configure(config)` (line 78): Configure pytest with custom markers and settings. -- `def pytest_collection_modifyitems(config, items)` (line 110): Automatically skip experiment and tutorial tests unless their opt-in flags -- `def pytest_runtest_logreport(report)` (line 158): Collect test timing information after each test completes. -- `def pytest_terminal_summary(terminalreporter, exitstatus, config)` (line 183): Print comprehensive test timing report after all tests complete. -- `def test_directories()` (line 345): Set up test directories for data and results. -- `def download_test_data(test_directories)` (line 370): Download Slicer-Heart-CT data. -- `def test_images(download_test_data, test_directories)` (line 397): Convert and resample 4D NRRD data; return pre-resampled time points. -- `def test_labelmaps(segmenter_total_segmentator, test_images, test_directories)` (line 450): Segment each time point with TotalSegmentator and return result dicts. -- `def test_transforms(registrar_ants, test_images, test_directories)` (line 491): Perform ANTs registration and return results. -- `def segmenter_total_segmentator()` (line 546): Create a SegmentChestTotalSegmentator instance. -- `def segmenter_simpleware()` (line 552): Create a SegmentHeartSimpleware instance. -- `def contour_tools()` (line 558): Create a ContourTools instance. -- `def registrar_ants()` (line 564): Create a RegisterImagesANTs instance. -- `def registrar_greedy()` (line 570): Create a RegisterImagesGreedy instance. -- `def registrar_icon()` (line 576): Create a RegisterImagesICON instance. -- `def transform_tools()` (line 582): Create a TransformTools instance. +- `def pytest_addoption(parser)` (line 53): Add custom command-line options for pytest. +- `def pytest_configure(config)` (line 111): Configure pytest with custom markers and settings. +- `def pytest_collection_modifyitems(config, items)` (line 148): Automatically skip experiment and tutorial tests unless their opt-in flags +- `def pytest_runtest_logreport(report)` (line 213): Collect test timing information after each test completes. +- `def pytest_terminal_summary(terminalreporter, exitstatus, config)` (line 238): Print comprehensive test timing report after all tests complete. +- `def test_directories()` (line 400): Set up test directories for data and results. +- `def download_test_data(test_directories)` (line 425): Download Slicer-Heart-CT data. +- `def test_images(download_test_data, test_directories)` (line 452): Convert and resample 4D NRRD data; return pre-resampled time points. +- `def test_labelmaps(segmenter_total_segmentator, test_images, test_directories)` (line 505): Segment each time point with TotalSegmentator and return result dicts. +- `def test_transforms(registrar_ants, test_images, test_directories)` (line 546): Perform ANTs registration and return results. +- `def segmenter_total_segmentator()` (line 601): Create a SegmentChestTotalSegmentator instance. +- `def segmenter_simpleware()` (line 607): Create a SegmentHeartSimpleware instance. +- `def contour_tools()` (line 613): Create a ContourTools instance. +- `def registrar_ants()` (line 619): Create a RegisterImagesANTs instance. +- `def registrar_greedy()` (line 625): Create a RegisterImagesGreedy instance. +- `def registrar_icon()` (line 631): Create a RegisterImagesICON instance. +- `def transform_tools()` (line 637): Create a TransformTools instance. ## tests/test_anatomy_taxonomy.py @@ -1040,21 +1086,22 @@ _Re-run `py utils/generate_api_map.py` whenever public APIs change._ - **class TestRegisterTimeSeriesImages** (line 24): Test suite for time series image registration. - `def test_registrar_initialization_ants(self)` (line 29): Test that RegisterTimeSeriesImages initializes correctly with ANTs. - `def test_registrar_initialization_icon(self)` (line 43): Test that RegisterTimeSeriesImages initializes correctly with ICON. - - `def test_registrar_initialization_invalid_method(self)` (line 57): Test that invalid registration method raises error. - - `def test_set_modality(self)` (line 64): Test setting imaging modality. - - `def test_set_fixed_image(self, test_images)` (line 72): Test setting fixed image. - - `def test_set_number_of_iterations(self)` (line 83): Test setting number of iterations. - - `def test_register_time_series_basic(self, test_images, test_directories)` (line 103): Test basic time series registration without prior transform. - - `def test_register_time_series_with_prior(self, test_images, test_directories)` (line 185): Test time series registration with prior transform usage. - - `def test_register_time_series_identity_start(self, test_images)` (line 246): Test time series registration with identity for starting image. - - `def test_register_time_series_different_starting_indices(self, test_images)` (line 272): Test time series registration with different starting indices. - - `def test_register_time_series_error_no_fixed_image(self)` (line 302): Test that error is raised if fixed image not set. - - `def test_register_time_series_error_invalid_starting_index(self, test_images)` (line 313): Test that error is raised for invalid starting index. - - `def test_register_time_series_error_invalid_prior_portion(self, test_images)` (line 336): Test that error is raised for invalid prior portion value. - - `def test_transform_application_time_series(self, test_images, test_directories)` (line 361): Test applying transforms from time series registration. - - `def test_register_time_series_icon(self, test_images)` (line 413): Test time series registration with ICON method. - - `def test_register_time_series_with_mask(self, test_images, test_directories)` (line 438): Test time series registration with fixed image mask. - - `def test_bidirectional_registration(self, test_images)` (line 483): Test that bidirectional registration works correctly. + - `def test_registrar_initialization_greedy(self)` (line 57): Test that RegisterTimeSeriesImages initializes correctly with Greedy. + - `def test_registrar_initialization_invalid_method(self)` (line 73): Test that invalid registration method raises error. + - `def test_set_modality(self)` (line 80): Test setting imaging modality. + - `def test_set_fixed_image(self, test_images)` (line 88): Test setting fixed image. + - `def test_set_number_of_iterations(self)` (line 99): Test setting number of iterations. + - `def test_register_time_series_basic(self, test_images, test_directories)` (line 127): Test basic time series registration without prior transform. + - `def test_register_time_series_with_prior(self, test_images, test_directories)` (line 209): Test time series registration with prior transform usage. + - `def test_register_time_series_identity_start(self, test_images)` (line 270): Test time series registration with identity for starting image. + - `def test_register_time_series_different_starting_indices(self, test_images)` (line 296): Test time series registration with different starting indices. + - `def test_register_time_series_error_no_fixed_image(self)` (line 326): Test that error is raised if fixed image not set. + - `def test_register_time_series_error_invalid_starting_index(self, test_images)` (line 337): Test that error is raised for invalid starting index. + - `def test_register_time_series_error_invalid_prior_portion(self, test_images)` (line 360): Test that error is raised for invalid prior portion value. + - `def test_transform_application_time_series(self, test_images, test_directories)` (line 385): Test applying transforms from time series registration. + - `def test_register_time_series_icon(self, test_images)` (line 437): Test time series registration with ICON method. + - `def test_register_time_series_with_mask(self, test_images, test_directories)` (line 462): Test time series registration with fixed image mask. + - `def test_bidirectional_registration(self, test_images)` (line 507): Test that bidirectional registration works correctly. ## tests/test_segment_chest_total_segmentator.py @@ -1196,11 +1243,11 @@ _Re-run `py utils/generate_api_map.py` whenever public APIs change._ - `def fetch_reviews(pr_number, repo)` (line 459) - `def resolve_review_threads(thread_ids, repo)` (line 464): Mark each thread in *thread_ids* as resolved via the GitHub GraphQL API. - `def build_prompt(pr_number, pr_data, reviews, thread_comments, summary_filename, agent, repo_root)` (line 580) -- `def invoke_ai_agent(prompt, repo_root, agent)` (line 708): Invoke the selected AI agent non-interactively. -- `def invoke_claude(prompt, repo_root)` (line 718): Invoke Claude Code non-interactively via stdin. -- `def invoke_codex(prompt, repo_root)` (line 750): Invoke Codex CLI non-interactively. -- `def parse_args()` (line 823) -- `def main()` (line 894) +- `def invoke_ai_agent(prompt, repo_root, agent)` (line 712): Invoke the selected AI agent non-interactively. +- `def invoke_claude(prompt, repo_root)` (line 722): Invoke Claude Code non-interactively via stdin. +- `def invoke_codex(prompt, repo_root)` (line 754): Invoke Codex CLI non-interactively. +- `def parse_args()` (line 827) +- `def main()` (line 898) ## utils/generate_api_map.py diff --git a/docs/api/registration/time_series.rst b/docs/api/registration/time_series.rst index 20ccfdd..5a0a09a 100644 --- a/docs/api/registration/time_series.rst +++ b/docs/api/registration/time_series.rst @@ -6,7 +6,8 @@ Time-Series Registration .. currentmodule:: physiomotion4d ``RegisterTimeSeriesImages`` registers ordered 3D image phases to a reference -frame using ANTs, ICON, or the combined ``ants_icon`` method. +frame using ANTs, Greedy, ICON, or combined ``ants_icon`` / ``greedy_icon`` +methods. Class Reference =============== @@ -43,5 +44,6 @@ See Also ======== * :doc:`ants` +* :doc:`greedy` * :doc:`icon` * :doc:`../../cli_scripts/4dct_reconstruction` diff --git a/docs/contributing.rst b/docs/contributing.rst index a37d0c2..3b86eec 100644 --- a/docs/contributing.rst +++ b/docs/contributing.rst @@ -267,8 +267,9 @@ Run Tests # Opt into specific buckets pytest tests/ --run-slow pytest tests/ --run-gpu --run-slow # typical local GPU profile - # Self-hosted CI GPU runner enables every bucket: - # --run-gpu --run-slow --run-simpleware --run-experiments --run-tutorials + pytest tests/ --run-physicsnemo # needs [physicsnemo] extra installed + # --run-all turns on every --run-* bucket at once (used by self-hosted CI): + pytest tests/ --run-all Documentation ============= diff --git a/docs/developer/core.rst b/docs/developer/core.rst index 73167ef..6d461fb 100644 --- a/docs/developer/core.rst +++ b/docs/developer/core.rst @@ -50,9 +50,10 @@ For most code changes, run: py -m pytest tests/ -v -(Slow / GPU / Simpleware / experiment / tutorial tests are auto-skipped; -opt in with ``--run-slow``, ``--run-gpu``, ``--run-simpleware``, -``--run-experiments``, ``--run-tutorials``. Data-dependent tests download +(Slow / GPU / Simpleware / PhysicsNeMo / experiment / tutorial tests are +auto-skipped; opt in with ``--run-slow``, ``--run-gpu``, ``--run-simpleware``, +``--run-physicsnemo``, ``--run-experiments``, ``--run-tutorials``, or use +``--run-all`` to enable every bucket at once. Data-dependent tests download their data through the session fixtures and run by default.) After public API changes, regenerate the API map: diff --git a/docs/quickstart.rst b/docs/quickstart.rst index 182a26c..2faf4e2 100644 --- a/docs/quickstart.rst +++ b/docs/quickstart.rst @@ -247,8 +247,9 @@ Download Sample Cardiac CT Data DirLab-4DCT data is manual-only; see ``data/README.md`` before running the high-resolution 4D CT reconstruction, lung-lobe PCA model, or PCA time-series -propagation tutorials. Tutorial 9 also requires the PhysicsNeMo dependency -installed with PhysioMotion4D. +propagation tutorials. Tutorial 9 additionally requires the optional +``physicsnemo`` extra (``pip install "physiomotion4d[physicsnemo]"``); +PhysicsNeMo itself requires Python >= 3.11. Visualizing Results =================== diff --git a/docs/testing.rst b/docs/testing.rst index fe25efb..2b00c34 100644 --- a/docs/testing.rst +++ b/docs/testing.rst @@ -22,6 +22,7 @@ Each ``--run-`` flag enables one marker family: pytest tests/ -v --run-slow # tests marked 'slow' pytest tests/ -v --run-gpu # tests marked 'requires_gpu' pytest tests/ -v --run-simpleware # tests marked 'requires_simpleware' + pytest tests/ -v --run-physicsnemo # tests marked 'requires_physicsnemo' pytest tests/ -v --run-experiments # tests marked 'experiment' pytest tests/ -v --run-tutorials # tests marked 'tutorial' @@ -31,11 +32,13 @@ Flags compose. A typical local GPU profile is: pytest tests/ -v --run-gpu --run-slow -The self-hosted CI GPU runner enables every bucket: +``--run-all`` is a convenience flag that turns on every ``--run-*`` bucket at +once. The self-hosted CI GPU runner uses it (after installing +``.[test,cuda13,physicsnemo]``): .. code-block:: bash - pytest tests/ -v --run-gpu --run-slow --run-simpleware --run-experiments --run-tutorials + pytest tests/ -v --run-all Test Categories =============== @@ -72,11 +75,11 @@ licensed Simpleware Medical installation locally). Continuous Integration ====================== -CI runs the fast subset by default. The self-hosted GPU runner invokes pytest -with every opt-in flag enabled -(``--run-gpu --run-slow --run-simpleware --run-experiments --run-tutorials``); -tests whose host requirements aren't met (e.g. a licensed Simpleware install -on a runner without one) runtime-skip cleanly via their internal guards. +CI runs the fast subset by default. The self-hosted GPU runner installs +``.[test,cuda13,physicsnemo]`` and invokes pytest with ``--run-all`` (which +enables every ``--run-*`` bucket); tests whose host requirements aren't met +(e.g. a licensed Simpleware install on a runner without one) runtime-skip +cleanly via their internal guards. See Also ======== diff --git a/docs/tutorials.rst b/docs/tutorials.rst index 0634ee8..4c76264 100644 --- a/docs/tutorials.rst +++ b/docs/tutorials.rst @@ -271,6 +271,10 @@ Workflow Dataset Tutorial 8 propagated PCA mesh outputs. +Extra install + PhysicsNeMo is an optional dependency. Install with + ``pip install "physiomotion4d[physicsnemo]"`` (requires Python >= 3.11). + Run .. code-block:: bash diff --git a/pyproject.toml b/pyproject.toml index b8a9d80..a7715c0 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -14,7 +14,7 @@ maintainers = [ ] readme = {file = "README.md", content-type = "text/markdown"} license = {text = "Apache-2.0"} -requires-python = ">=3.11" +requires-python = ">=3.10" classifiers = [ "Development Status :: 4 - Beta", "Intended Audience :: Healthcare Industry", @@ -22,6 +22,7 @@ classifiers = [ "License :: OSI Approved :: Apache Software License", "Operating System :: OS Independent", "Programming Language :: Python :: 3", + "Programming Language :: Python :: 3.10", "Programming Language :: Python :: 3.11", "Programming Language :: Python :: 3.12", "Topic :: Scientific/Engineering :: Medical Science Apps.", @@ -63,9 +64,8 @@ dependencies = [ # AI/ML and segmentation "monai>=1.3.0", - "nvidia-physicsnemo>=2.0.0,<3.0.0", - "torch>=2.0.0,<3.0.0", - "transformers>=4.21.0,<5.0.0", + "torch>=2.0.0", + "transformers>=4.21.0", "totalsegmentator>=2.0.0", # Registration @@ -106,10 +106,17 @@ explicit = true [project.optional-dependencies] cuda13 = [ "cupy-cuda13x>=13.6.0", - "torch>=2.0.0,<3.0.0", + "torch>=2.0.0", "torchvision", "torchaudio", ] +# PhysicsNeMo pulls in a large dependency stack (CUDA-only toolchain, narrower +# Python support). It is only needed for Tutorial 9 (mesh stage MLP). Install +# with ``pip install physiomotion4d[physicsnemo]`` when you need it. Note: +# nvidia-physicsnemo itself requires Python >= 3.11. +physicsnemo = [ + "nvidia-physicsnemo>=2.0.0", +] dev = [ "bumpver>=2023.0.0", "jupytext>=1.16.0", @@ -192,7 +199,7 @@ push = false ] [tool.mypy] -python_version = "3.11" +python_version = "3.10" warn_return_any = true warn_unused_configs = true disallow_untyped_defs = true @@ -286,6 +293,7 @@ markers = [ "slow: marks tests as slow (skipped by default; opt-in via --run-slow)", "requires_gpu: marks tests that require GPU/CUDA support (opt-in via --run-gpu)", "requires_simpleware: marks tests that require a local Synopsys Simpleware Medical installation (opt-in via --run-simpleware)", + "requires_physicsnemo: marks tests that require the optional [physicsnemo] extra (opt-in via --run-physicsnemo)", "experiment: marks tests that run experiment scripts (extremely slow, manual only)", "tutorial: marks tests that run tutorial scripts (data/GPU gated, manual only)", "xdist_group: marks tests to run in the same pytest-xdist worker (prevents parallel conflicts)" diff --git a/src/physiomotion4d/register_time_series_images.py b/src/physiomotion4d/register_time_series_images.py index c6e0f99..8f2e0c0 100644 --- a/src/physiomotion4d/register_time_series_images.py +++ b/src/physiomotion4d/register_time_series_images.py @@ -1,9 +1,8 @@ """Time series image registration implementation. This module provides the RegisterTimeSeriesImages class for registering an ordered -sequence of images (time series) to a fixed image. It supports both ANTs -and ICON registration methods and can optionally use prior transforms to initialize -subsequent registrations in the sequence. +sequence of images (time series) to a fixed image. It supports ANTs, Greedy, +ICON, and combined ANTs/Greedy initialization followed by ICON refinement. The class is particularly useful for 4D medical imaging applications such as cardiac CT where sequential frames need to be registered to a common frame. @@ -16,17 +15,21 @@ from physiomotion4d.register_images_ants import RegisterImagesANTs from physiomotion4d.register_images_base import RegisterImagesBase +from physiomotion4d.register_images_greedy import RegisterImagesGreedy from physiomotion4d.register_images_icon import RegisterImagesICON from physiomotion4d.transform_tools import TransformTools +REGISTRATION_METHODS = ["ants", "greedy", "icon", "ants_icon", "greedy_icon"] + class RegisterTimeSeriesImages(RegisterImagesBase): """Register a time series of images to a fixed image. This class extends RegisterImagesBase to provide sequential registration of - multiple images (time series) to a fixed image. It supports both - ANTs and ICON registration methods and can propagate information from prior - registrations to initialize subsequent ones. + multiple images (time series) to a fixed image. It supports ANTs, Greedy, + ICON, and combined ANTs/Greedy initialization followed by ICON refinement. + It can propagate information from prior registrations to initialize + subsequent ones. The registration proceeds in two passes from a reference frame: 1. Forward pass: from reference_frame to the end of the series @@ -37,13 +40,13 @@ class RegisterTimeSeriesImages(RegisterImagesBase): Key features: - Sequential registration of ordered image lists - - Support for both ANTs and ICON registration backends + - Support for ANTs, Greedy, ICON, ANTs+ICON, and Greedy+ICON backends - Optional use of prior transforms to initialize next registration - Configurable starting point in the time series - Returns all transforms and loss values for the entire series Attributes: - registration_method (str): Registration method to use ('ants' or 'icon') + registration_method (str): Registration method to use registrar (RegisterImagesBase): Internal registration object (ANTs or ICON) transform_tools (TransformTools): Utility for transform operations @@ -81,26 +84,30 @@ def __init__( Args: registration_method (str): Registration method to use. - Options: 'ants' or 'icon'. Default: 'ants' + Options: 'ants', 'greedy', 'icon', 'ants_icon', or + 'greedy_icon'. Default: 'ants' log_level: Logging level (default: logging.INFO) Raises: - ValueError: If registration_method is not 'ants' or 'icon' + ValueError: If registration_method is not supported. """ super().__init__(log_level=log_level) self.registration_method_name: str = registration_method.lower() self.registrar_ants = RegisterImagesANTs(log_level=log_level) + self.registrar_greedy = RegisterImagesGreedy(log_level=log_level) self.registrar_icon = RegisterImagesICON(log_level=log_level) # Set default iterations based on registration method self.number_of_iterations_ants: list[int] = [40, 20, 10] + self.number_of_iterations_greedy: list[int] = [40, 20, 10] self.number_of_iterations_icon: int = 50 - if self.registration_method_name not in ["ants", "icon", "ants_icon"]: + if self.registration_method_name not in REGISTRATION_METHODS: raise ValueError( - f"registration_method must be 'ants', 'icon' or 'ants_icon', got '{registration_method}'" + "registration_method must be one of " + f"{REGISTRATION_METHODS}, got '{registration_method}'" ) self.transform_tools: TransformTools = TransformTools() @@ -126,6 +133,17 @@ def set_number_of_iterations_icon(self, number_of_iterations_icon: int) -> None: """ self.number_of_iterations_icon = number_of_iterations_icon + def set_number_of_iterations_greedy( + self, number_of_iterations_greedy: list[int] + ) -> None: + """Set the number of iterations for Greedy registration. + + Args: + number_of_iterations_greedy: List of iterations per Greedy + resolution level, for example ``[40, 20, 10]``. + """ + self.number_of_iterations_greedy = number_of_iterations_greedy + def set_smooth_prior_transform_sigma( self, smooth_prior_transform_sigma: float ) -> None: @@ -273,6 +291,15 @@ def register_time_series( self.registrar_ants.set_number_of_iterations(self.number_of_iterations_ants) self.registrar_ants.set_fixed_mask(self.fixed_mask) self.registrar_ants.set_fixed_labelmap(self.fixed_labelmap) + elif self.registration_method_name == "greedy": + self.registrar_greedy.set_fixed_image(self.fixed_image) + self.registrar_greedy.set_modality(self.modality) + self.registrar_greedy.set_mask_dilation(self.mask_dilation_mm) + self.registrar_greedy.set_number_of_iterations( + self.number_of_iterations_greedy + ) + self.registrar_greedy.set_fixed_mask(self.fixed_mask) + self.registrar_greedy.set_fixed_labelmap(self.fixed_labelmap) elif self.registration_method_name == "icon": self.registrar_icon.set_fixed_image(self.fixed_image) self.registrar_icon.set_modality(self.modality) @@ -280,13 +307,25 @@ def register_time_series( self.registrar_icon.set_number_of_iterations(self.number_of_iterations_icon) self.registrar_icon.set_fixed_mask(self.fixed_mask) self.registrar_icon.set_fixed_labelmap(self.fixed_labelmap) - elif self.registration_method_name == "ants_icon": - self.registrar_ants.set_fixed_image(self.fixed_image) - self.registrar_ants.set_modality(self.modality) - self.registrar_ants.set_mask_dilation(self.mask_dilation_mm) - self.registrar_ants.set_number_of_iterations(self.number_of_iterations_ants) - self.registrar_ants.set_fixed_mask(self.fixed_mask) - self.registrar_ants.set_fixed_labelmap(self.fixed_labelmap) + elif self.registration_method_name in ["ants_icon", "greedy_icon"]: + if self.registration_method_name == "ants_icon": + self.registrar_ants.set_fixed_image(self.fixed_image) + self.registrar_ants.set_modality(self.modality) + self.registrar_ants.set_mask_dilation(self.mask_dilation_mm) + self.registrar_ants.set_number_of_iterations( + self.number_of_iterations_ants + ) + self.registrar_ants.set_fixed_mask(self.fixed_mask) + self.registrar_ants.set_fixed_labelmap(self.fixed_labelmap) + else: + self.registrar_greedy.set_fixed_image(self.fixed_image) + self.registrar_greedy.set_modality(self.modality) + self.registrar_greedy.set_mask_dilation(self.mask_dilation_mm) + self.registrar_greedy.set_number_of_iterations( + self.number_of_iterations_greedy + ) + self.registrar_greedy.set_fixed_mask(self.fixed_mask) + self.registrar_greedy.set_fixed_labelmap(self.fixed_labelmap) self.registrar_icon.set_fixed_image(self.fixed_image) self.registrar_icon.set_modality(self.modality) self.registrar_icon.set_mask_dilation(self.mask_dilation_mm) @@ -345,24 +384,35 @@ def register_time_series( moving_mask=reference_mask, moving_labelmap=reference_labelmap, ) + elif self.registration_method_name == "greedy": + result = self.registrar_greedy.register( + moving_images[reference_frame], + moving_mask=reference_mask, + moving_labelmap=reference_labelmap, + ) elif self.registration_method_name == "icon": result = self.registrar_icon.register( moving_images[reference_frame], moving_mask=reference_mask, moving_labelmap=reference_labelmap, ) - elif self.registration_method_name == "ants_icon": - result = self.registrar_ants.register( + elif self.registration_method_name in ["ants_icon", "greedy_icon"]: + registrar_initial = ( + self.registrar_ants + if self.registration_method_name == "ants_icon" + else self.registrar_greedy + ) + result = registrar_initial.register( moving_images[reference_frame], moving_mask=reference_mask, moving_labelmap=reference_labelmap, ) - forward_ants = result["forward_transform"] + forward_initial = result["forward_transform"] result = self.registrar_icon.register( moving_images[reference_frame], moving_mask=reference_mask, moving_labelmap=reference_labelmap, - initial_forward_transform=forward_ants, + initial_forward_transform=forward_initial, ) else: raise ValueError( @@ -420,24 +470,35 @@ def register_time_series( moving_mask=moving_mask, moving_labelmap=moving_labelmap, ) + elif self.registration_method_name == "greedy": + result_init_identity = self.registrar_greedy.register( + moving_image=moving_image, + moving_mask=moving_mask, + moving_labelmap=moving_labelmap, + ) elif self.registration_method_name == "icon": result_init_identity = self.registrar_icon.register( moving_image=moving_image, moving_mask=moving_mask, moving_labelmap=moving_labelmap, ) - elif self.registration_method_name == "ants_icon": - result_init_identity = self.registrar_ants.register( + elif self.registration_method_name in ["ants_icon", "greedy_icon"]: + registrar_initial = ( + self.registrar_ants + if self.registration_method_name == "ants_icon" + else self.registrar_greedy + ) + result_init_identity = registrar_initial.register( moving_image=moving_image, moving_mask=moving_mask, moving_labelmap=moving_labelmap, ) - forward_ants = result_init_identity["forward_transform"] + forward_initial = result_init_identity["forward_transform"] result_init_identity = self.registrar_icon.register( moving_image=moving_image, moving_mask=moving_mask, moving_labelmap=moving_labelmap, - initial_forward_transform=forward_ants, + initial_forward_transform=forward_initial, ) else: raise ValueError( @@ -457,6 +518,13 @@ def register_time_series( moving_labelmap=moving_labelmap, initial_forward_transform=prior_forward, ) + elif self.registration_method_name == "greedy": + result_init_prior = self.registrar_greedy.register( + moving_image=moving_image, + moving_mask=moving_mask, + moving_labelmap=moving_labelmap, + initial_forward_transform=prior_forward, + ) elif self.registration_method_name == "icon": result_init_prior = self.registrar_icon.register( moving_image=moving_image, @@ -464,19 +532,24 @@ def register_time_series( moving_labelmap=moving_labelmap, initial_forward_transform=prior_forward, ) - elif self.registration_method_name == "ants_icon": - result_init_prior = self.registrar_ants.register( + elif self.registration_method_name in ["ants_icon", "greedy_icon"]: + registrar_initial = ( + self.registrar_ants + if self.registration_method_name == "ants_icon" + else self.registrar_greedy + ) + result_init_prior = registrar_initial.register( moving_image=moving_image, moving_mask=moving_mask, moving_labelmap=moving_labelmap, initial_forward_transform=prior_forward, ) - forward_ants = result_init_prior["forward_transform"] + forward_initial = result_init_prior["forward_transform"] result_init_prior = self.registrar_icon.register( moving_image=moving_image, moving_mask=moving_mask, moving_labelmap=moving_labelmap, - initial_forward_transform=forward_ants, + initial_forward_transform=forward_initial, ) else: raise ValueError( @@ -696,6 +769,19 @@ def registration_method( "inverse_transform": cast(itk.Transform, res["inverse_transform"]), "loss": float(cast(float, res["loss"])), } + if self.registration_method_name == "greedy": + res = self.registrar_greedy.registration_method( + moving_image=moving_image, + moving_mask=moving_mask, + moving_labelmap=moving_labelmap, + moving_image_pre=moving_image_pre, + initial_forward_transform=initial_forward_transform, + ) + return { + "forward_transform": cast(itk.Transform, res["forward_transform"]), + "inverse_transform": cast(itk.Transform, res["inverse_transform"]), + "loss": float(cast(float, res["loss"])), + } if self.registration_method_name == "icon": res = self.registrar_icon.registration_method( moving_image=moving_image, @@ -709,20 +795,25 @@ def registration_method( "inverse_transform": cast(itk.Transform, res["inverse_transform"]), "loss": float(cast(float, res["loss"])), } - if self.registration_method_name == "ants_icon": - ants_res = self.registrar_ants.registration_method( + if self.registration_method_name in ["ants_icon", "greedy_icon"]: + registrar_initial = ( + self.registrar_ants + if self.registration_method_name == "ants_icon" + else self.registrar_greedy + ) + initial_res = registrar_initial.registration_method( moving_image=moving_image, moving_mask=moving_mask, moving_labelmap=moving_labelmap, moving_image_pre=moving_image_pre, ) - forward_ants = ants_res["forward_transform"] + forward_initial = initial_res["forward_transform"] icon_res = self.registrar_icon.registration_method( moving_image=moving_image, moving_mask=moving_mask, moving_labelmap=moving_labelmap, moving_image_pre=moving_image_pre, - initial_forward_transform=forward_ants, + initial_forward_transform=forward_initial, ) return { "forward_transform": cast(itk.Transform, icon_res["forward_transform"]), diff --git a/tests/README.md b/tests/README.md index ba588ef..4876470 100644 --- a/tests/README.md +++ b/tests/README.md @@ -104,17 +104,21 @@ Each flag enables one marker family. Flags compose, so you can stack them. | `--run-slow` | `slow` | Slow registration / segmentation tests (>30 s) | | `--run-gpu` | `requires_gpu` | CUDA-dependent tests (ICON, Simpleware, etc.) | | `--run-simpleware` | `requires_simpleware` | Need a licensed Synopsys Simpleware Medical install (also marked `requires_gpu`) | +| `--run-physicsnemo` | `requires_physicsnemo` | Need the optional `[physicsnemo]` extra installed | | `--run-experiments` | `experiment` | End-to-end experiment notebooks (hours to run) | | `--run-tutorials` | `tutorial` | Tutorial scripts run end-to-end | +| `--run-all` | every bucket above | Equivalent to passing all `--run-*` flags at once | ```bash # Slow tests pytest tests/ -v --run-slow -# Typical local GPU profile. The self-hosted CI GPU runner enables every -# bucket: --run-gpu --run-slow --run-simpleware --run-experiments --run-tutorials +# Typical local GPU profile. The self-hosted CI GPU runner uses --run-all. pytest tests/ -v --run-gpu --run-slow +# Enable every bucket at once +pytest tests/ -v --run-all + # Full Simpleware coverage (requires Simpleware Medical installed locally) pytest tests/ -v --run-simpleware --run-gpu --run-slow @@ -156,10 +160,15 @@ All test runs automatically generate a comprehensive timing report at the end sh - `@pytest.mark.requires_simpleware` — Tests needing a licensed Synopsys Simpleware Medical install. Opt in: `--run-simpleware`. (Combine with `--run-gpu` and `--run-slow`.) +- `@pytest.mark.requires_physicsnemo` — Tests needing the optional + `[physicsnemo]` extra (`pip install "physiomotion4d[physicsnemo]"`, requires + Python >= 3.11). Opt in: `--run-physicsnemo`. - `@pytest.mark.experiment` — End-to-end experiment notebooks (EXTREMELY SLOW, never in CI). Opt in: `--run-experiments`. - `@pytest.mark.tutorial` — Tutorial scripts run end-to-end. Opt in: `--run-tutorials`. + +`--run-all` is a convenience flag that turns on every `--run-*` bucket at once. - `@pytest.mark.integration` — Integration tests vs unit tests (filter-only). - `@pytest.mark.timeout(seconds)` — Per-test timeout override. diff --git a/tests/conftest.py b/tests/conftest.py index 50955da..290faad 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -32,6 +32,24 @@ _pytest_config: Optional[pytest.Config] = None +_RUN_BUCKET_FLAGS = ( + "--run-experiments", + "--run-tutorials", + "--run-simpleware", + "--run-slow", + "--run-gpu", + "--run-physicsnemo", +) + + +def _run_bucket_enabled(config: pytest.Config, flag: str) -> bool: + """Return True if ``flag`` (a --run-* bucket) is on, directly or via --run-all.""" + return bool( + config.getoption(flag, default=False) + or config.getoption("--run-all", default=False), + ) + + def pytest_addoption(parser: pytest.Parser) -> None: """Add custom command-line options for pytest.""" parser.addoption( @@ -67,6 +85,21 @@ def pytest_addoption(parser: pytest.Parser) -> None: default=False, help="Run tests marked 'requires_gpu' (skipped by default)", ) + parser.addoption( + "--run-physicsnemo", + action="store_true", + default=False, + help=( + "Run tests marked 'requires_physicsnemo' (need the optional " + "[physicsnemo] extra installed)" + ), + ) + parser.addoption( + "--run-all", + action="store_true", + default=False, + help=("Enable every --run-* bucket: " + ", ".join(_RUN_BUCKET_FLAGS) + "."), + ) parser.addoption( "--create-baselines", action="store_true", @@ -99,6 +132,11 @@ def pytest_configure(config: pytest.Config) -> None: "requires_simpleware: marks tests that need a local Synopsys Simpleware " "Medical installation (skipped unless --run-simpleware is passed)", ) + config.addinivalue_line( + "markers", + "requires_physicsnemo: marks tests that need the optional " + "[physicsnemo] extra installed (skipped unless --run-physicsnemo is passed)", + ) # Initialize test timing storage config._test_timings = { # type: ignore[attr-defined] "tests": [], @@ -118,39 +156,56 @@ def pytest_collection_modifyitems( accidentally when running the normal test suite. """ for item in items: - if "experiment" in item.keywords and not config.getoption("--run-experiments"): + if "experiment" in item.keywords and not _run_bucket_enabled( + config, "--run-experiments" + ): item.add_marker( pytest.mark.skip( - reason="Experiment tests require --run-experiments flag to run" + reason="Experiment tests require --run-experiments (or --run-all) to run" ) ) - if "tutorial" in item.keywords and not config.getoption("--run-tutorials"): + if "tutorial" in item.keywords and not _run_bucket_enabled( + config, "--run-tutorials" + ): item.add_marker( pytest.mark.skip( - reason="Tutorial tests require --run-tutorials flag to run" + reason="Tutorial tests require --run-tutorials (or --run-all) to run" ) ) - if "requires_simpleware" in item.keywords and not config.getoption( - "--run-simpleware" + if "requires_simpleware" in item.keywords and not _run_bucket_enabled( + config, "--run-simpleware" ): item.add_marker( pytest.mark.skip( reason=( - "Simpleware tests require --run-simpleware flag and a " - "local Synopsys Simpleware Medical installation" + "Simpleware tests require --run-simpleware (or --run-all) " + "and a local Synopsys Simpleware Medical installation" ) ) ) - if "slow" in item.keywords and not config.getoption("--run-slow"): + if "slow" in item.keywords and not _run_bucket_enabled(config, "--run-slow"): item.add_marker( pytest.mark.skip( - reason="Slow tests require --run-slow flag to run", + reason="Slow tests require --run-slow (or --run-all) to run", ) ) - if "requires_gpu" in item.keywords and not config.getoption("--run-gpu"): + if "requires_gpu" in item.keywords and not _run_bucket_enabled( + config, "--run-gpu" + ): item.add_marker( pytest.mark.skip( - reason="GPU tests require --run-gpu flag to run", + reason="GPU tests require --run-gpu (or --run-all) to run", + ) + ) + if "requires_physicsnemo" in item.keywords and not _run_bucket_enabled( + config, "--run-physicsnemo" + ): + item.add_marker( + pytest.mark.skip( + reason=( + "PhysicsNeMo tests require --run-physicsnemo (or --run-all) " + "and the optional [physicsnemo] extra installed" + ) ) ) diff --git a/tests/test_register_time_series_images.py b/tests/test_register_time_series_images.py index a58a6b7..d37d053 100644 --- a/tests/test_register_time_series_images.py +++ b/tests/test_register_time_series_images.py @@ -54,6 +54,22 @@ def test_registrar_initialization_icon(self) -> None: print("\nTime series registrar initialized with ICON") + def test_registrar_initialization_greedy(self) -> None: + """Test that RegisterTimeSeriesImages initializes correctly with Greedy.""" + registrar = RegisterTimeSeriesImages(registration_method="greedy") + assert registrar is not None, "Registrar not initialized" + assert registrar.registration_method_name == "greedy", ( + "Method not set correctly" + ) + assert registrar.registrar_greedy is not None, ( + "Internal Greedy registrar not created" + ) + assert registrar.registrar_icon is not None, ( + "Internal ICON registrar not created" + ) + + print("\nTime series registrar initialized with Greedy") + def test_registrar_initialization_invalid_method(self) -> None: """Test that invalid registration method raises error.""" with pytest.raises(ValueError, match="registration_method must be"): @@ -90,6 +106,14 @@ def test_set_number_of_iterations(self) -> None: "ANTs iterations not set correctly" ) + registrar_greedy = RegisterTimeSeriesImages(registration_method="greedy") + iterations_greedy = [25, 10, 3] + + registrar_greedy.set_number_of_iterations_greedy(iterations_greedy) + assert registrar_greedy.number_of_iterations_greedy == iterations_greedy, ( + "Greedy iterations not set correctly" + ) + registrar_icon = RegisterTimeSeriesImages(registration_method="icon") iterations_icon = 50 diff --git a/tutorials/README.md b/tutorials/README.md index 805988d..6357e2b 100644 --- a/tutorials/README.md +++ b/tutorials/README.md @@ -21,7 +21,7 @@ dataset licensing, and expected directory layout. | 6 | [tutorial_06_reconstruct_highres_4d_ct.py](tutorial_06_reconstruct_highres_4d_ct.py) | `WorkflowReconstructHighres4DCT` | DirLab-4DCT (manual) | | 7 | [tutorial_07_dirlab_pca_model.py](tutorial_07_dirlab_pca_model.py) | `WorkflowCreateStatisticalModel`, `WorkflowFitStatisticalModelToPatient` | DirLab-4DCT (manual) | | 8 | [tutorial_08_dirlab_pca_time_series.py](tutorial_08_dirlab_pca_time_series.py) | `RegisterTimeSeriesImages` | DirLab-4DCT plus Tutorial 7 output | -| 9 | [tutorial_09_physicsnemo_mesh_stage_model.py](tutorial_09_physicsnemo_mesh_stage_model.py) | `physicsnemo.models.mlp.FullyConnected` | Tutorial 8 output | +| 9 | [tutorial_09_physicsnemo_mesh_stage_model.py](tutorial_09_physicsnemo_mesh_stage_model.py) | `physicsnemo.models.mlp.FullyConnected` (requires `[physicsnemo]` extra) | Tutorial 8 output | ## Running a Tutorial @@ -72,7 +72,7 @@ pytest tests/test_tutorials.py::TestTutorial01HeartGatedCTToUSD --run-tutorials 5. **Tutorial 6** requires DirLab-4DCT - download it per `data/README.md`. 6. **Tutorial 7** creates a surface PCA model of the five lung lobes from DirLab-4DCT, then fits it to every available case. 7. **Tutorial 8** registers DirLab respiratory phases with ANTs+ICON and propagates the Tutorial 7 fitted meshes through each time series. -8. **Tutorial 9** trains a PhysicsNeMo model to predict a PCA-fitted mesh at a user-specified respiratory stage. +8. **Tutorial 9** trains a PhysicsNeMo model to predict a PCA-fitted mesh at a user-specified respiratory stage. PhysicsNeMo is an optional extra: install with `pip install "physiomotion4d[physicsnemo]"` (requires Python >= 3.11). ## For Contributors diff --git a/tutorials/tutorial_09_physicsnemo_mesh_stage_model.py b/tutorials/tutorial_09_physicsnemo_mesh_stage_model.py index 3a83dc5..7e23521 100644 --- a/tutorials/tutorial_09_physicsnemo_mesh_stage_model.py +++ b/tutorials/tutorial_09_physicsnemo_mesh_stage_model.py @@ -11,6 +11,14 @@ ------------- Run Tutorial 8 first so ``output/tutorial_08_dirlab_pca_time_series`` contains ``Case*/meshes/*_pca_fit.vtp`` files. + +Extra Install Required +---------------------- +PhysicsNeMo is an optional dependency of PhysioMotion4D. Install it with:: + + pip install "physiomotion4d[physicsnemo]" + +PhysicsNeMo itself requires Python >= 3.11. """ # %% @@ -24,7 +32,16 @@ import numpy as np import pyvista as pv import torch -from physicsnemo.models.mlp import FullyConnected + + +try: + from physicsnemo.models.mlp import FullyConnected +except ImportError as exc: # pragma: no cover - import-time guard + raise ImportError( + "Tutorial 9 requires PhysicsNeMo, which is an optional dependency. " + 'Install with: pip install "physiomotion4d[physicsnemo]" ' + "(requires Python >= 3.11).", + ) from exc # nnUNetv2 (used by TotalSegmentator inside several workflows) spawns a diff --git a/utils/ai_agent_github_reviews.py b/utils/ai_agent_github_reviews.py index 367d90a..de34ea8 100644 --- a/utils/ai_agent_github_reviews.py +++ b/utils/ai_agent_github_reviews.py @@ -655,7 +655,11 @@ def build_prompt( from `PhysioMotion4DBase`; helper/data/container classes need not - Adds features or abstractions beyond what was requested - Calls `vtk_to_usd` internals from outside `convert_vtk_to_usd.py` - - Applies coordinate conversion (RAS->Y-up) more than once + - Applies coordinate conversion (LPS->USD Y-up) more than once, or + treats internal PyVista surfaces as RAS (they are LPS) + - Uses emojis in `.py` files + - Omits the Windows `if __name__ == "__main__":` guard in scripts that + instantiate `SegmentChestTotalSegmentator` - Exceeds 88-character line length ## Step 3 — Write summary From e1adcc0ca31bcbab7c1155a58c47df51fa758b44 Mon Sep 17 00:00:00 2001 From: Stephen Aylward Date: Sat, 23 May 2026 10:37:18 -0400 Subject: [PATCH 2/3] BUG: Updated from review-pr skill --- .agents/skills/check-conventions/SKILL.md | 2 +- .agents/skills/review-pr/SKILL.md | 5 ++- .github/workflows/README.md | 6 ++-- .github/workflows/ci.yml | 4 ++- .github/workflows/nightly-health.yml | 4 ++- .github/workflows/test-slow.yml | 4 ++- .gitignore | 4 +++ docs/API_MAP.md | 32 +++++++++---------- docs/contributing.rst | 2 +- .../register_time_series_images.py | 19 ++++++++--- utils/ai_agent_github_reviews.py | 7 +++- 11 files changed, 60 insertions(+), 29 deletions(-) diff --git a/.agents/skills/check-conventions/SKILL.md b/.agents/skills/check-conventions/SKILL.md index dc1823f..f7b788b 100644 --- a/.agents/skills/check-conventions/SKILL.md +++ b/.agents/skills/check-conventions/SKILL.md @@ -64,7 +64,7 @@ context such as class inheritance), then flag every occurrence of: Group findings by file. For each finding, print: -``` +```text : ``` diff --git a/.agents/skills/review-pr/SKILL.md b/.agents/skills/review-pr/SKILL.md index 54fcdd3..bbaf219 100644 --- a/.agents/skills/review-pr/SKILL.md +++ b/.agents/skills/review-pr/SKILL.md @@ -73,7 +73,10 @@ Defaults worth knowing: working-tree changes only; the user controls staging (`git add -p`) and commit. -4. If the run failed: +4. Delete `pr__review_summary.md` after reporting — the working-tree + diff is the durable record; the summary is intermediate state. + +5. If the run failed: - "gh CLI not found" → tell the user to install via `winget install GitHub.cli` then `gh auth login`. - "claude CLI not found" → tell the user to install via diff --git a/.github/workflows/README.md b/.github/workflows/README.md index 9f3fb5b..ec1fdc1 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -144,7 +144,8 @@ GPU tests require self-hosted runners with: **Option 3: Run Locally** ```bash -# Install with CUDA + PhysicsNeMo (matches the self-hosted GPU runner) +# Install with CUDA + PhysicsNeMo (matches the self-hosted GPU runner). +# Requires Python >= 3.11 (nvidia-physicsnemo does not support 3.10). uv pip install -e ".[test,cuda13,physicsnemo]" # Run GPU tests @@ -217,7 +218,8 @@ pytest tests/ -m "unit and not requires_gpu" --cov=physiomotion4d ### GPU Tests ```bash -# Install with CUDA + PhysicsNeMo (matches the self-hosted GPU runner) +# Install with CUDA + PhysicsNeMo (matches the self-hosted GPU runner). +# Requires Python >= 3.11 (nvidia-physicsnemo does not support 3.10). uv pip install -e ".[test,cuda13,physicsnemo]" # Run GPU tests diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3ccb7f6..97f2aeb 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -282,8 +282,10 @@ jobs: lfs: true - name: Create venv in RUNNER_TEMP + # Python 3.11 is required because the [physicsnemo] extra pulls in + # nvidia-physicsnemo, which requires Python >= 3.11. run: | - & "C:\Program Files\Python310\python.exe" -m venv "$env:RUNNER_TEMP\physiomotion4d-venv" + & "C:\Program Files\Python311\python.exe" -m venv "$env:RUNNER_TEMP\physiomotion4d-venv" echo "$env:RUNNER_TEMP\physiomotion4d-venv\Scripts" >> $env:GITHUB_PATH - name: Check GPU availability diff --git a/.github/workflows/nightly-health.yml b/.github/workflows/nightly-health.yml index 53e31c0..d486c0f 100644 --- a/.github/workflows/nightly-health.yml +++ b/.github/workflows/nightly-health.yml @@ -55,8 +55,10 @@ jobs: run: nvidia-smi - name: Create venv in RUNNER_TEMP + # Python 3.11 is required because the [physicsnemo] extra pulls in + # nvidia-physicsnemo, which requires Python >= 3.11. run: | - & "C:\Program Files\Python310\python.exe" -m venv "$env:RUNNER_TEMP\physiomotion4d-venv" + & "C:\Program Files\Python311\python.exe" -m venv "$env:RUNNER_TEMP\physiomotion4d-venv" echo "$env:RUNNER_TEMP\physiomotion4d-venv\Scripts" >> $env:GITHUB_PATH - name: Cache uv packages diff --git a/.github/workflows/test-slow.yml b/.github/workflows/test-slow.yml index be9035e..8b84fdf 100644 --- a/.github/workflows/test-slow.yml +++ b/.github/workflows/test-slow.yml @@ -21,8 +21,10 @@ jobs: lfs: true - name: Create venv in RUNNER_TEMP + # Python 3.11 is required because the [physicsnemo] extra pulls in + # nvidia-physicsnemo, which requires Python >= 3.11. run: | - & "C:\Program Files\Python310\python.exe" -m venv "$env:RUNNER_TEMP\physiomotion4d-venv" + & "C:\Program Files\Python311\python.exe" -m venv "$env:RUNNER_TEMP\physiomotion4d-venv" echo "$env:RUNNER_TEMP\physiomotion4d-venv\Scripts" >> $env:GITHUB_PATH - name: Check GPU availability diff --git a/.gitignore b/.gitignore index 9af39e1..8993f8e 100644 --- a/.gitignore +++ b/.gitignore @@ -17,6 +17,10 @@ htmlcov # AI files .claude +# Intermediate files produced by skills +.github_reveiw_prompt.txt +pr*review_summary.md + # Documentation build files docs/_build/ docs/_static/.buildinfo diff --git a/docs/API_MAP.md b/docs/API_MAP.md index f1f04e3..43f9ba6 100644 --- a/docs/API_MAP.md +++ b/docs/API_MAP.md @@ -83,7 +83,7 @@ _Re-run `py utils/generate_api_map.py` whenever public APIs change._ - `def write_summary(path, rows)` (line 366): Write the wide-form summary CSV. - `def mask_from_labelmap(labelmap)` (line 378): Return a uint8 binary mask covering the non-zero labels. - `def run_method_for_subject(subject_id, timepoint_artifacts, reference_index, method_spec, output_dir, icon_iterations, run_resegmentation, error_detail_file)` (line 386): Run one ICON method for one subject and return per-timepoint rows. -- `def main()` (line 535): Run the ICON default-vs-finetuned comparison experiment. +- `def main()` (line 544): Run the ICON default-vs-finetuned comparison experiment. ## experiments/LongitudinalRegistration/recon_4d_run.py @@ -643,19 +643,19 @@ _Re-run `py utils/generate_api_map.py` whenever public APIs change._ ## src/physiomotion4d/register_time_series_images.py -- **class RegisterTimeSeriesImages** (line 25): Register a time series of images to a fixed image. - - `def __init__(self, registration_method='ants', log_level=logging.INFO)` (line 80): Initialize the time series image registration class. - - `def set_number_of_iterations_ants(self, number_of_iterations_ants)` (line 117): Set the number of iterations for ANTs registration. - - `def set_number_of_iterations_icon(self, number_of_iterations_icon)` (line 128): Set the number of iterations for ICON registration. - - `def set_number_of_iterations_greedy(self, number_of_iterations_greedy)` (line 136): Set the number of iterations for Greedy registration. - - `def set_smooth_prior_transform_sigma(self, smooth_prior_transform_sigma)` (line 147): Set the sigma for smoothing the prior transform. - - `def set_mask_dilation(self, mask_dilation_mm)` (line 157): Set the dilation of the fixed and moving image masks. - - `def set_modality(self, modality)` (line 167): Set the imaging modality for registration optimization. - - `def set_fixed_image(self, fixed_image)` (line 177): Set the fixed image for registration. - - `def set_fixed_mask(self, fixed_mask)` (line 188): Set a binary mask for the fixed image region of interest. - - `def register_time_series(self, moving_images, moving_masks=None, moving_labelmaps=None, reference_frame=0, register_reference=True, prior_weight=0.0)` (line 198): Register a time series of images to the fixed image. - - `def reconstruct_time_series(self, moving_images, inverse_transforms, upsample_to_fixed_resolution=False)` (line 607): Reconstruct time series images using inverse transforms. - - `def registration_method(self, moving_image, moving_mask=None, moving_labelmap=None, moving_image_pre=None, initial_forward_transform=None)` (line 737): Registration method required by RegisterImagesBase. +- **class RegisterTimeSeriesImages** (line 31): Register a time series of images to a fixed image. + - `def __init__(self, registration_method='ants', log_level=logging.INFO)` (line 90): Initialize the time series image registration class. + - `def set_number_of_iterations_ants(self, number_of_iterations_ants)` (line 127): Set the number of iterations for ANTs registration. + - `def set_number_of_iterations_icon(self, number_of_iterations_icon)` (line 138): Set the number of iterations for ICON registration. + - `def set_number_of_iterations_greedy(self, number_of_iterations_greedy)` (line 146): Set the number of iterations for Greedy registration. + - `def set_smooth_prior_transform_sigma(self, smooth_prior_transform_sigma)` (line 157): Set the sigma for smoothing the prior transform. + - `def set_mask_dilation(self, mask_dilation_mm)` (line 167): Set the dilation of the fixed and moving image masks. + - `def set_modality(self, modality)` (line 177): Set the imaging modality for registration optimization. + - `def set_fixed_image(self, fixed_image)` (line 187): Set the fixed image for registration. + - `def set_fixed_mask(self, fixed_mask)` (line 198): Set a binary mask for the fixed image region of interest. + - `def register_time_series(self, moving_images, moving_masks=None, moving_labelmaps=None, reference_frame=0, register_reference=True, prior_weight=0.0)` (line 208): Register a time series of images to the fixed image. + - `def reconstruct_time_series(self, moving_images, inverse_transforms, upsample_to_fixed_resolution=False)` (line 617): Reconstruct time series images using inverse transforms. + - `def registration_method(self, moving_image, moving_mask=None, moving_labelmap=None, moving_image_pre=None, initial_forward_transform=None)` (line 747): Registration method required by RegisterImagesBase. ## src/physiomotion4d/segment_anatomy_base.py @@ -1246,8 +1246,8 @@ _Re-run `py utils/generate_api_map.py` whenever public APIs change._ - `def invoke_ai_agent(prompt, repo_root, agent)` (line 712): Invoke the selected AI agent non-interactively. - `def invoke_claude(prompt, repo_root)` (line 722): Invoke Claude Code non-interactively via stdin. - `def invoke_codex(prompt, repo_root)` (line 754): Invoke Codex CLI non-interactively. -- `def parse_args()` (line 827) -- `def main()` (line 898) +- `def parse_args()` (line 832) +- `def main()` (line 903) ## utils/generate_api_map.py diff --git a/docs/contributing.rst b/docs/contributing.rst index 3b86eec..4c62e97 100644 --- a/docs/contributing.rst +++ b/docs/contributing.rst @@ -267,7 +267,7 @@ Run Tests # Opt into specific buckets pytest tests/ --run-slow pytest tests/ --run-gpu --run-slow # typical local GPU profile - pytest tests/ --run-physicsnemo # needs [physicsnemo] extra installed + pytest tests/ --run-physicsnemo # needs [physicsnemo] extra; requires Python >= 3.11 # --run-all turns on every --run-* bucket at once (used by self-hosted CI): pytest tests/ --run-all diff --git a/src/physiomotion4d/register_time_series_images.py b/src/physiomotion4d/register_time_series_images.py index 8f2e0c0..349e4d6 100644 --- a/src/physiomotion4d/register_time_series_images.py +++ b/src/physiomotion4d/register_time_series_images.py @@ -19,7 +19,13 @@ from physiomotion4d.register_images_icon import RegisterImagesICON from physiomotion4d.transform_tools import TransformTools -REGISTRATION_METHODS = ["ants", "greedy", "icon", "ants_icon", "greedy_icon"] +REGISTRATION_METHODS: list[str] = [ + "ants", + "greedy", + "icon", + "ants_icon", + "greedy_icon", +] class RegisterTimeSeriesImages(RegisterImagesBase): @@ -46,9 +52,13 @@ class RegisterTimeSeriesImages(RegisterImagesBase): - Returns all transforms and loss values for the entire series Attributes: - registration_method (str): Registration method to use - registrar (RegisterImagesBase): Internal registration object (ANTs or ICON) - transform_tools (TransformTools): Utility for transform operations + registration_method_name (str): Registration method in use ('ants', + 'greedy', 'icon', 'ants_icon', or 'greedy_icon'). + registrar_ants (RegisterImagesANTs): Internal ANTs registrar. + registrar_greedy (RegisterImagesGreedy): Internal Greedy registrar. + registrar_icon (RegisterImagesICON): Internal ICON registrar (also used + as the refinement stage for 'ants_icon' and 'greedy_icon'). + transform_tools (TransformTools): Utility for transform operations. Example: >>> # Register a cardiac CT time series @@ -806,6 +816,7 @@ def registration_method( moving_mask=moving_mask, moving_labelmap=moving_labelmap, moving_image_pre=moving_image_pre, + initial_forward_transform=initial_forward_transform, ) forward_initial = initial_res["forward_transform"] icon_res = self.registrar_icon.registration_method( diff --git a/utils/ai_agent_github_reviews.py b/utils/ai_agent_github_reviews.py index de34ea8..8252bff 100644 --- a/utils/ai_agent_github_reviews.py +++ b/utils/ai_agent_github_reviews.py @@ -796,6 +796,11 @@ def invoke_codex(prompt: str, repo_root: Path) -> None: _save_prompt_fallback(prompt, repo_root, agent="codex") sys.exit(exc.returncode) + # Prompt file is intermediate input to the agent; remove on success. + # On failure (FileNotFoundError / CalledProcessError) the fallback path + # above re-saves it for manual rerun, so we only delete here. + prompt_path.unlink(missing_ok=True) + def _save_prompt_file(prompt: str, repo_root: Path) -> Path: fallback = repo_root / ".github_review_prompt.txt" @@ -1023,7 +1028,7 @@ def main() -> None: print() summary_path = repo_root / summary_filename if summary_path.exists(): - print(f"[✓] Summary written : {summary_filename}") + print(f"[+] Summary written : {summary_filename}") else: print("[!] Summary file not found — check agent output above.") print("[*] Inspect changes : git diff") From 0dd33444bd307573d3d29da524d508bdb2b9129f Mon Sep 17 00:00:00 2001 From: Stephen Aylward Date: Sat, 23 May 2026 12:44:00 -0400 Subject: [PATCH 3/3] STYLE: Typo in .gitignore --- .gitignore | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index 8993f8e..be88ce1 100644 --- a/.gitignore +++ b/.gitignore @@ -18,7 +18,7 @@ htmlcov .claude # Intermediate files produced by skills -.github_reveiw_prompt.txt +.github_review_prompt.txt pr*review_summary.md # Documentation build files