diff --git a/.gitignore b/.gitignore index 98e67cba2..47713a7ec 100644 --- a/.gitignore +++ b/.gitignore @@ -38,6 +38,9 @@ dist/ # --- C++ / CMAKE --- # Exclude all build directories except for the base folder if needed build*/ +# `build*/` also matches documentation directories whose name starts with "build" +# (e.g. skills/build-and-ci/). Keep those tracked. +!skills/build*-*/ CMakeFiles/ CMakeCache.txt CMakeUserPresets.json @@ -83,5 +86,10 @@ opengate/tests/log a.txt *.so +# --- LOCAL MACHINE CONFIG (never committed) --- +# Per-machine absolute paths and credentials read by agents/humans. +user_secrets.json +user_secrets.*.json + .clangd diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 000000000..3a72c296f --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,202 @@ +# AGENTS.md — working on the OpenGATE/opengate repository + +This file is the **entry point for any AI agent** (or human in a hurry) that has to +modify, test, debug or document this repository. Read it fully before doing anything. +Then open the specific skill you need from [`skills/`](skills/). + +> Project: **GATE 10** — Monte Carlo simulation platform for medical physics +> (PET, SPECT, CT, external/internal radiotherapy, dosimetry), built on Geant4. +> Version: see [`VERSION`](VERSION) (currently `10.1.1`). +> Requires Python >= 3.10 (CI tests 3.10 → 3.14). Geant4 `v11.4.2`, ITK `v5.4.4`. + +--- + +## 1. Repository layout (what lives where) + +| Path | Content | +| --- | --- | +| `opengate/` | Pure-Python package: user API, managers, engines, actors, sources, geometry, physics, image handling. This is `import opengate`. | +| `opengate/tests/` | Test suite. `src/` contains `testXXX_*.py` scripts, `utility.py` helpers, `data/` (git submodule with binary test data), `log/`, `output/` (generated). | +| `opengate/bin/` | Console entry points (`opengate_tests`, `opengate_info`, `opengate_jobs_*`, `opengate_visu`, …). Declared in `pyproject.toml`. | +| `core/` | C++ subpackage `opengate_core` — pybind11 bindings + Gate C++ library binding Geant4 and ITK. Compiled with CMake. | +| `core/external/` | Vendored submodules: `pybind11`, `fmt`. | +| `docs/` | Sphinx sources (`docs/source/user_guide`, `docs/source/developer_guide`). Published on Read the Docs. | +| `.github/workflows/` | CI: `main.yml` (orchestrator), `actions_build/` (wheel builds), `actions_tests/` (test run + dashboard upload). | +| `pyproject.toml` | Metadata of the Python package `opengate`. | +| `core/setup.py`, `core/pyproject.toml` | Metadata + CMake build of `opengate_core`. | +| `VERSION` | Single source of truth of the version; `opengate` requires `opengate-core==`. | +| `AGENTS.md`, `skills/` | Agent instructions and skills (this directory). | +| `user_secrets.json` | **Git-ignored, per-machine** absolute paths (`OPEN_GATE_REPO`, `OPEN_GATE_ENV`, `OPEN_GATE_DEPS`). Read by agents before building/running. Create it and ask the user for the values — see `skills/environment-setup` §0.1. | + +Two-package design: `opengate` (Python, fast to install) depends on `opengate_core` +(C++, ships Geant4 + ITK binaries in the published wheels). A **developer install must +compile `opengate_core` locally** — the pip-distributed `opengate_core` wheel does not +match local C++ changes. + +## 2. Golden rules + +1. **Never commit on `master`.** Do all work on a dedicated branch, and make sure it is + **based on the up-to-date local `master`** before you start (see §6). `master` must stay + byte-identical to upstream; if you find yourself on `master` with changes, move them to a + branch (`git switch -c `) before doing anything else. +2. **Never commit without explicit user approval.** Stage and show what would be committed, + then ask. Do not `git commit`, `git rebase`, `git push`, `git reset --hard` or otherwise + rewrite/advance history on your own initiative — the user validates the changes first. +3. **Never run tests against a half-installed tree.** Use the user-designated virtual + environment on **non-volatile** storage (`$OPEN_GATE_ENV`) and *always* run the suite + via the `opengate_tests` entry point (never bare `python testXXX.py` loops, never + `pytest` — this repo uses its own runner). Paths come from `user_secrets.json` (see + below); never invent them. See [`skills/environment-setup/SKILL.md`](skills/environment-setup/SKILL.md). +4. **Machine-specific values live in `user_secrets.json`** at the root of the repo. It is + **git-ignored — never commit it**, never paste its contents into a committed file, test, + doc page or commit message. If it is missing, create it from the template in + [`skills/environment-setup/SKILL.md`](skills/environment-setup/SKILL.md) §0.1 **and ask + the user for the real values** (never guess a path). +5. **Submodules first.** `git submodule update --init --recursive` in `$OPEN_GATE_REPO` + before building or running tests. Missing `opengate/tests/data` = most tests fail for + the wrong reason. +6. **`--recurse-submodules` on clone**, `git lfs` must be installed (test data is large binary). +7. **Formatting is enforced** by `pre-commit` (black for Python, clang-format for C++, + trailing whitespace). Run it before committing — see [`skills/code-style/SKILL.md`](skills/code-style/SKILL.md). +8. **Version bump = 3 places**: `VERSION` at root, and the release must republish both + `opengate` and `opengate_core` (`opengate-core==10.1.1` is a hard pin in `setup.py`). +9. **Never commit generated artifacts**: `opengate/tests/output*/`, `opengate/tests/log/`, + `dist/`, `build/`, `*.egg-info`, images/root files (see `.gitignore`). +10. **Do not let tests pollute the tree.** Running the suite can leave untracked files in + the repository root (e.g. `simulation.json`, see B-006). Check `git status` after every + run and remove them — never include them in a commit. +11. **Keep `skills/status/found-bugs.md` up to date** whenever you discover a bug you do + not immediately fix, and `skills/status/task-list.md` when you plan/close work. +12. **Distinguish repository bugs from environment issues.** A wrong Geant4/ITK version, a + stale compiled build or an un-activated venv is **not** a repo bug — fix the environment + (see `skills/environment-setup`). Only log a repository bug once the environment has been + proven correct. +13. **Do not run the whole suite casually**: it takes tens of minutes, downloads Geant4 + data on first run, and needs ~all cores. Use `--start_id/--end_id`/`-t` filters. +14. **Cite real evidence.** Do not claim a test passed unless you ran it (or read the CI + result). Do not invent Geant4/actor parameter names — grep the code and docs first. + +## 3. Skills index + +Map of the `skills/` directory: [`skills/README.md`](skills/README.md). + +| Skill | Use it when | +| --- | --- | +| [`skills/environment-setup/`](skills/environment-setup/SKILL.md) | You need a working virtual dev environment where **all tests** run (venv, Geant4/ITK, `pip install -e .`, optional extras) — or you are **releasing a version**, touching wheels/packaging metadata, or editing `.github/workflows/` (deep-dive: [`build-and-ci.md`](skills/environment-setup/build-and-ci.md)). | +| [`skills/running-tests/`](skills/running-tests/SKILL.md) | You must run, filter, debug or interpret the test suite and its dashboard output. | +| [`skills/code-style/`](skills/code-style/SKILL.md) | Before every commit: formatting, naming, imports, docstrings. | +| [`skills/architecture/`](skills/architecture/SKILL.md) | You add/modify engines, managers, actors, sources, physics — the object lifecycle in GATE 10. | +| [`skills/documentation/`](skills/documentation/SKILL.md) | You add a user/developer guide page or build the Sphinx docs. | +| [`skills/engineering/`](skills/engineering/SKILL.md) | You are acting in a role: `python-gate-developer`, `geant4-physics-expert`, `gate-physics-expert`, `gate-chemistry-expert`, `geant4-geometry-expert`, `gate-geometry-expert`, `simulation-debugger`, `test-writer`. | +| [`skills/status/`](skills/status/task-list.md) | You need the shared task list, the bug log, or the project status snapshot. | + +## 4. Quick start (30-second version) + +**Before anything else, get the machine's paths from `user_secrets.json`** (root of the +repo, git-ignored). If it does not exist, create it from the template in +[`skills/environment-setup/SKILL.md`](skills/environment-setup/SKILL.md) §0.1 and **ask the +user for the real values**. The environment **must be on non-volatile storage** (not +`/tmp`, a tmpfs, or a container layer): the C++ build and the downloaded Geant4 data take +minutes and GBs and must survive a reboot. **Reuse an existing environment if one already +works**; only create one when none does. `skills/environment-setup` §0 is machine-agnostic +— it never hard-codes a path. + +```bash +# 0. load the user's paths from user_secrets.json (create it first if missing) +cd "$OPEN_GATE_REPO" +export OPEN_GATE_ENV="$(python3 -c 'import json;print(json.load(open("user_secrets.json"))["OPEN_GATE_ENV"])')" + +# 1. submodules (git-lfs required) +git lfs install && git submodule update --init --recursive + +# 2. environment — create only if no usable one exists (environment-setup §0.3–0.4) +uv venv "$OPEN_GATE_ENV" # or: python3 -m venv "$OPEN_GATE_ENV" +source "$OPEN_GATE_ENV/bin/activate" + +# 3. Python package, editable (uses the pip-provided opengate_core wheel) +python -m pip install -e . # uv env: VIRTUAL_ENV="$OPEN_GATE_ENV" uv pip install -e . + +# 4. smoke test — KEEP THE ENV ACTIVATED (the runner spawns `python `), +# and the -t path is relative to opengate/tests/src INCLUDING the subdirectory +# (first run downloads Geant4 data on import — can take several minutes) +opengate_tests -t actors/test008_dose_actor.py +``` + +For anything involving `core/` (C++), Qt visualization, or a *reproducible* full-suite +run, follow [`skills/environment-setup/SKILL.md`](skills/environment-setup/SKILL.md) +which builds Geant4/ITK and `opengate_core` (ask the user for the deps prefix too). + +## 5. Hierarchy of truth + +When sources disagree, trust them in this order: + +1. the code in `opengate/` and `core/` (runtime behaviour); +2. the test suite `opengate/tests/src/` (executable specification); +3. `.github/workflows/` (how the maintainers actually build & test); +4. `docs/source/` (may lag behind, especially `DesignGuidelines.md` which is marked **OBSOLETE**); +5. this file and `skills/` (may lag behind the code — fix them if you spot drift). + +## 6. Git workflow — do this before starting any work + +**`master` is read-only for agents.** Never commit, merge into, rebase onto, or push +`master`. The only thing an agent may do with it is **sync it from the remotes**, and every +change an agent makes must live on a **fresh branch based on the up-to-date local `master`**. + +This repo has two remotes; check yours with `git remote -v`: + +| Remote | Role | +| --- | --- | +| `origin` | the user's fork (where the agent's branches are pushed, only with user approval) | +| `opengate` | upstream `OpenGATE/opengate` (the source of truth) | + +### 6.1 Sync (run at the start of every session and before creating a branch) + +```bash +cd "$OPEN_GATE_REPO" + +# 1. never work on master: get off it first if you are on it +[ "$(git branch --show-current)" = master ] && git switch - # or: git switch + +# 2. update the local master from the remotes (fast-forward only, no history rewrite) +git fetch origin +git fetch opengate +git switch master +git merge --ff-only origin/master # local master must match origin/master +git merge --ff-only opengate/master # and upstream (both should already be identical) +``` + +Verify all three are in sync — each count must be `0 0`: + +```bash +git rev-list --left-right --count master...origin/master # expect: 0 0 +git rev-list --left-right --count master...opengate/master # expect: 0 0 +git rev-list --left-right --count origin/master...opengate/master # expect: 0 0 +``` + +If a `--ff-only` merge refuses, **stop and ask the user**: local `master` has diverged from +upstream, and resolving that is the user's decision (do not rebase or reset it yourself). + +### 6.2 Create the branch — always fresh, always from master + +```bash +git switch -c - master # e.g. fix-doseactor-scaling +``` + +Rules: + +- **One branch per task**, cut from the current `master` tip. Never reuse an old branch for a + new task, and never branch off another working branch. +- If you are already on a branch, verify it is based on the *current* master tip before adding + commits: + ```bash + git merge-base --is-ancestor master HEAD && echo "based on master: OK" + git log --oneline master..HEAD # your commits — should be only your own work + ``` +- If `master` has advanced since you branched, **rebase onto it** rather than merging: + ```bash + git rebase master # only if the branch has no shared/upstream commits, and only + # after telling the user — see golden rules + ``` +- **Never** `git push --force` to `master` or to a shared branch. +- Commit only what your task needs; stage explicitly (`git add `), never `git add -A` + blindly. diff --git a/core/setup.py b/core/setup.py index 0a5d446bc..3bf015fd0 100755 --- a/core/setup.py +++ b/core/setup.py @@ -97,6 +97,16 @@ def build_extension(self, ext): # cmake_args += ['-DCMAKE_CXX_FLAGS="-Wno-self-assign -Wno-extra-semi"'] cmake_args += ["-DCMAKE_EXPORT_COMPILE_COMMANDS=ON"] + # Number of parallel compile jobs. Historically this was pinned to 4, which made + # a cold build needlessly slow on any modern machine. Default to all available + # cores and allow an explicit override via the standard environment variable. + try: + num_jobs = int(os.environ.get("OPEN_GATE_BUILD_JOBS", "")) + if num_jobs < 1: + raise ValueError + except ValueError: + num_jobs = os.cpu_count() or 4 + if platform.system() == "Windows": cmake_args += [ "-DCMAKE_LIBRARY_OUTPUT_DIRECTORY_{}={}".format(cfg.upper(), extdir) @@ -109,11 +119,11 @@ def build_extension(self, ext): cmake_args += ["-A", "x64"] build_args += ["--", "/m"] else: - build_args += ["--parallel", "4"] + build_args += ["--parallel", str(num_jobs)] else: cmake_args += ['-DCMAKE_CXX_FLAGS="-Wno-pedantic"'] cmake_args += ["-DCMAKE_BUILD_TYPE=" + cfg] - build_args += ["--", "-j4"] + build_args += ["--", f"-j{num_jobs}"] env["CXXFLAGS"] = '{} -DVERSION_INFO=\\"{}\\"'.format( env.get("CXXFLAGS", ""), self.distribution.get_version() diff --git a/skills/README.md b/skills/README.md new file mode 100644 index 000000000..51fb57b6e --- /dev/null +++ b/skills/README.md @@ -0,0 +1,152 @@ +# `skills/` — agent playbooks for the OpenGATE/opengate repository + +Task-oriented instructions for AI agents (and humans in a hurry) working on GATE 10. +**Start with [`../AGENTS.md`](../AGENTS.md)** — it holds the repository layout, the golden +rules and the quick start. This README is the map of the skills themselves. + +Each file is self-contained: read the one you need, not the whole directory. + +--- + +## Layout + +``` +skills/ +├── README.md ← this file +├── environment-setup/ +│ ├── SKILL.md get a working dev env where ALL tests run (index) +│ ├── geant4-itk.md build Geant4/ITK + rebuild the opengate_core extension +│ └── build-and-ci.md packaging, releasing, CI topology, reproduce CI locally +├── running-tests/SKILL.md the opengate_tests runner, filters, dashboard +├── code-style/SKILL.md pre-commit / black / clang-format, conventions +├── architecture/SKILL.md managers / engines / actors / sources lifecycle +├── documentation/SKILL.md Sphinx pages and local build +├── engineering/ +│ ├── SKILL.md pick a role + map of the role skills +│ ├── python-gate-developer.md write Python API code that fits the design +│ ├── geant4-physics-expert.md Geant4 physics: lists, EM models, cuts, statistics +│ ├── gate-physics-expert.md GATE physics layer: sources, actors, filters +│ ├── gate-chemistry-expert.md GATE chemistry layer: Geant4-DNA chemistry, world, counters +│ ├── geant4-geometry-expert.md raw Geant4 geometry: solids, volumes, navigation +│ ├── gate-geometry-expert.md GATE geometry layer: volume tree, solids, materials +│ ├── simulation-debugger.md diagnose crashes / hangs / implausible output +│ └── test-writer.md write tests that actually catch regressions +└── status/ + ├── task-list.md shared work list (id, status, area, evidence) + ├── found-bugs.md bug log: repository bugs vs environment issues + └── project-status.md dated snapshot of the project and its health +``` + +## Which skill do I need? + +| Situation | Read | +| --- | --- | +| No environment yet, or "tests fail for no reason" | [`environment-setup/SKILL.md`](environment-setup/SKILL.md) | +| Need to run, filter, or interpret tests | [`running-tests/SKILL.md`](running-tests/SKILL.md) | +| **Releasing**, changing packaging/wheels, or editing `.github/workflows/` | [`environment-setup/build-and-ci.md`](environment-setup/build-and-ci.md) | +| **Building Geant4/ITK or the `core/` C++ extension** | [`environment-setup/geant4-itk.md`](environment-setup/geant4-itk.md) | +| About to commit | [`code-style/SKILL.md`](code-style/SKILL.md) | +| Adding an actor / source / engine / manager | [`architecture/SKILL.md`](architecture/SKILL.md) | +| Adding or changing a docs page | [`documentation/SKILL.md`](documentation/SKILL.md) | +| Writing or refactoring `opengate/**` Python code | [`engineering/python-gate-developer.md`](engineering/python-gate-developer.md) | +| Choosing a physics list, cuts, or judging a result | [`engineering/geant4-physics-expert.md`](engineering/geant4-physics-expert.md) | +| Configuring the physics manager, sources, or actors | [`engineering/gate-physics-expert.md`](engineering/gate-physics-expert.md) | +| Configuring or debugging Geant4-DNA **chemistry** (lists, world, scavengers, chemistry actors) | [`engineering/gate-chemistry-expert.md`](engineering/gate-chemistry-expert.md) | +| Adding/moving volumes, solids, materials, fields | [`engineering/gate-geometry-expert.md`](engineering/gate-geometry-expert.md) | +| G4-level geometry, overlaps, navigation, new bindings | [`engineering/geant4-geometry-expert.md`](engineering/geant4-geometry-expert.md) | +| Not sure which engineering role fits | [`engineering/SKILL.md`](engineering/SKILL.md) | +| A simulation crashes, hangs, or gives implausible output | [`engineering/simulation-debugger.md`](engineering/simulation-debugger.md) | +| Adding a test | [`engineering/test-writer.md`](engineering/test-writer.md) | +| Planning work, logging a bug, checking project state | [`status/`](status/task-list.md) | + +### Easily-confused pairs — one owner per fact + +These skills look overlapping from their titles but own different questions. When in doubt, +read the **owner** column, and follow its links rather than duplicating the content. + +| Question | Owner | Not the owner | +| --- | --- | --- | +| "My local setup is broken / how do I make the suite run?" | [`environment-setup`](environment-setup/SKILL.md) §0–§5, §7–§9 | — | +| "How do I ship this / why does CI build wheels?" | [`environment-setup/build-and-ci.md`](environment-setup/build-and-ci.md) | — | +| "How must the code *look* (format, naming, imports)?" | [`code-style`](code-style/SKILL.md) | `architecture`, `python-gate-developer` | +| "How must the code *behave* (lifecycle, `__initcpp__`, managers vs engines)?" | [`architecture`](architecture/SKILL.md) | `code-style` | + +Boundary rules the skills themselves enforce: + +- **Environment and build/CI/release are one skill family** (`environment-setup/`): `SKILL.md` + §0–§3/§5/§7–§9 set up your machine, and the two deep-dives own the heavy topics — + [`geant4-itk.md`](environment-setup/geant4-itk.md) for compiling the native deps and the C++ + extension, [`build-and-ci.md`](environment-setup/build-and-ci.md) for packaging, releasing and CI. + The former standalone `build-and-ci` skill was merged into `environment-setup` §6 and then + extracted back into its own file to keep `SKILL.md` an index. +- The **C++/Geant4 build procedure** lives in [`geant4-itk.md`](environment-setup/geant4-itk.md); + [`build-and-ci.md`](environment-setup/build-and-ci.md) §6 keeps only the packaging consequences. +- The **object-lifecycle rules** are owned by `architecture` §3/§7; the role skills restate + them for self-containment and link back for the rationale. + +## The engineering roles + +Eight role-oriented skills layer *on top of* the task-oriented ones above. They answer "how +should I think about this?" rather than "what command do I run?". **Start with +[`engineering/SKILL.md`](engineering/SKILL.md)** — it maps them, explains the two layered pairs, +and says which role to pick. + +| Role | Owns | Does **not** cover | +| --- | --- | --- | +| [`python-gate-developer.md`](engineering/python-gate-developer.md) | Python API design, managers/actors/sources, the C++ boundary, serialisation | physics validity, test design | +| [`geant4-physics-expert.md`](engineering/geant4-physics-expert.md) | Geant4 physics lists, EM models, cuts, statistics, physical plausibility | the GATE configuration layer, code structure | +| [`gate-physics-expert.md`](engineering/gate-physics-expert.md) | The GATE physics layer: physics manager, **sources**, **actors**, filters, biasing, normalisation | raw Geant4 model internals | +| [`gate-chemistry-expert.md`](engineering/gate-chemistry-expert.md) | The GATE chemistry layer: chemistry manager/list/world, track-structure EM regions, chemistry actors and counters | the general physics/source/actor machinery, raw Geant4-DNA model internals | +| [`geant4-geometry-expert.md`](engineering/geant4-geometry-expert.md) | Raw G4 geometry: solids, logical/physical volumes, materials, navigation, overlaps, C++ bindings | the GATE user-facing volume API | +| [`gate-geometry-expert.md`](engineering/gate-geometry-expert.md) | The GATE geometry layer: volume tree, solids, materials, placement/repetition, regions, fields | raw G4 navigation internals | +| [`simulation-debugger.md`](engineering/simulation-debugger.md) | Triage, reproduction, lifecycle-localised diagnosis | writing the fix's tests | +| [`test-writer.md`](engineering/test-writer.md) | Test placement, assertions, tolerances, demonstrating failure | the code under test | + +The two physics skills and the two geometry skills are **layered, not duplicated**: +`geant4-*-expert` covers the Geant4 model beneath GATE; `gate-*-expert` covers the GATE API +built on top of it (including actors and sources for physics). A change is *designed* with the +matching expert skill, *implemented* with the developer skill, *proved* with the test-writer +skill, and *triaged* with the debugger skill when it goes wrong. + +## Conventions every skill follows + +1. **Machine-agnostic paths.** No skill hard-codes an absolute path. Per-machine values come + from `user_secrets.json` at the repo root (git-ignored): + `OPEN_GATE_REPO`, `OPEN_GATE_ENV`, `OPEN_GATE_DEPS`. + See [`environment-setup/SKILL.md`](environment-setup/SKILL.md) §0. +2. **Classify before you diagnose.** Every failure is either a *repository bug* or an + *environment issue*. Most confusing ones are the latter. The distinction is enforced in + [`status/found-bugs.md`](status/found-bugs.md). +3. **Evidence over claims.** A command that was actually run, with its output — not "should + work". Test results come from `opengate_tests`, never from an absence of output. +4. **Checklists at the end.** Each skill ends with a "definition of done" / checklist; use it + before reporting completion. +5. **Skills can be wrong.** They describe the tree at the commit where they were written. If a + command in a skill fails, fix the skill in the same change + (`AGENTS.md` §5, hierarchy of truth). + +## Status files are part of the workflow + +- [`status/task-list.md`](status/task-list.md) — update the same turn you start or finish work. +- [`status/found-bugs.md`](status/found-bugs.md) — log anything you find but do not fix, with + the **category** filled in. +- [`status/project-status.md`](status/project-status.md) — a *dated* snapshot; correct it when + it drifts rather than letting it rot. + +## Adding a new skill +1. Put task skills in `skills//SKILL.md`; put role skills in `skills/engineering/`. +2. **`SKILL.md` is an index, not a textbook.** It states the goal, the scope, the map of where + things live, and the checklists — then hands off. Anything that needs real depth goes into a + dedicated `.md` next to it (the model is [`engineering/`](engineering/SKILL.md): a short + `SKILL.md` plus one file per role, and + [`environment-setup/`](environment-setup/SKILL.md): an index plus `geant4-itk.md` and + `build-and-ci.md`). If your `SKILL.md` passes ~250 lines, split it and repoint the refs — a + single 800-line file is read in full or not at all. +3. Follow the conventions above (no absolute paths, checklist at the end, cite real evidence). +3. **Add it to the tables in this README and to the skills index in + [`../AGENTS.md`](../AGENTS.md)** — an undocumented skill is invisible. +4. Keep it self-contained: another agent may read it in isolation. +5. **Check the file is actually trackable** — a skill directory whose name begins with + `build` is silently ignored by `.gitignore`'s `build*/` rule (B-011). Verify with + `git check-ignore -v skills//SKILL.md` (no output = fine) and confirm it appears in + `git status`. A skill that cannot be committed does not exist for anyone else. \ No newline at end of file diff --git a/skills/architecture/SKILL.md b/skills/architecture/SKILL.md new file mode 100644 index 000000000..9287e4929 --- /dev/null +++ b/skills/architecture/SKILL.md @@ -0,0 +1,143 @@ +# Skill: architecture + +**Goal:** add or modify an engine, manager, actor, source, filter or physics list +without violating the GATE 10 object lifecycle. + +`DesignGuidelines.md` at the repo root is marked **OBSOLETE** — do not rely on it as +documentation of the current design. Read the code in `opengate/` and the developer +guide pages under `docs/source/developer_guide/`. + +--- + +## 1. Mental model + +``` +Simulation (opengate/managers.py) <- the user-facing object + ├── VolumeManager, PhysicsManager, SourceManager, ActorManager, ... (user input + checks) + ├── Engines (opengate/engines.py) <- build/run Geant4 + └── GateObjects (opengate/base.py) <- serialisable units + ├── Actors (opengate/actors/**) + ├── Sources (opengate/sources/**) + ├── Volumes (opengate/geometry/**) + └── Fields, Filters, Phantom, ... +``` + +Two phases: + +1. **User phase** — managers receive and validate user input; nothing Geant4 is created. +2. **Initialization / simulation phase** — engines build the Geant4 world and objects + are initialized. + +**Managers check the user input; engines may assume it is valid.** Do not put user-input +validation in an engine, and do not create Geant4 objects during the user phase. + +## 2. Key files (go here first) + +| File | Role | +| --- | --- | +| `opengate/managers.py` | `Simulation` + all managers. Almost every user feature starts here. | +| `opengate/engines.py` | Run/init orchestration, subprocess handling, Geant4 bootstrap. | +| `opengate/base.py` | Base classes: `GateObject`, `UserElement`, `ActorBase`, deprecation helpers. | +| `opengate/actors/**` | Actors: `doseactors.py`, `digitizers.py`, `phspactors.py` (phase space), `arbitraryactors.py`, `filters.py`, `biasingactors.py`, `chemistryactors.py`, `coincidences.py`. | +| `opengate/sources/**` | `generic.py`, `phspsources.py`, `gansources.py`, `voxelsources.py`, `beamsources.py`, `phidsources.py`. | +| `opengate/geometry/**` | `volumes.py`, `solids.py`, `materials.py`, `fields.py`, `utility.py`. | +| `opengate/physics.py` | Physics lists and physics settings. | +| `opengate/userhooks.py`, `opengate/actions.py` | Where user hooks / G4 actions are registered. | +| `opengate/serialization.py`, `opengate/utility.py` | Serialisation (subprocesses!) and shared helpers. | +| `opengate_core` (C++, `core/`) | The Geant4-facing classes (`g4.GateXxx`). | + +## 3. Adding an actor — the mandatory class structure + +Per `docs/source/developer_guide/developer_guide_how_to_implement.rst`, an actor must: + +1. **Inherit the Python base class first, then the C++ base class**: + + ```python + class SimulationStatisticsActor(ActorBase, g4.GateSimulationStatisticsActor): + ... + ``` + + Never the reverse order. + +2. **Reference the superclass explicitly — do not** use `super()`, it cannot resolve + the C++ base class: + + ```python + ActorBase.initialize(self) + ``` + +3. **Implement `initialize()`** calling, at minimum: + + ```python + def initialize(self): + ActorBase.initialize(self) + # user-input sanity checks here, if any + self.InitializeUserInfo(self.user_info) + self.InitializeCpp() + ``` + +4. **Implement `__initcpp__()`** and call the C++ constructor **there**, never in + `__init__()`: + + ```python + def __initcpp__(self): + g4.GateSimulationStatisticsActor.__init__(self, self.user_info) + self.AddActions({"StartSimulationAction", "EndSimulationAction"}) + ``` + + Rationale: the multiprocessing mechanism de-/serialises objects, and + `__setstate__()` in `opengate/actors/base.py` relies on `__initcpp__` to rebuild the + C++ side in the subprocess. + +5. Constructor takes **no mandatory argument except the name**, and parameters are set + explicitly afterwards (this is also what makes dummy objects easy to build). + +6. Register the actor with the actor manager, expose the user-facing class in the + package namespace, and add tests (`skills/running-tests`) and docs + (`skills/documentation`). + +## 4. Adding a source + +Same pattern as actors, with `SourceBase` + the corresponding `g4.GateXxxSource`. Sources +live in `opengate/sources/`; the user-facing creation goes through the source manager in +`opengate/managers.py`. Respect the "no mandatory `__init__` args" rule, and keep +randomness/seed handling consistent with the existing sources. + +## 5. Managers and engines + +- Registries are **dicts of the form `{name: object}`**, not lists — adding objects must + not change insertion-order semantics that other code depends on. +- Manager methods validate types/sizes/ranges and raise the project exceptions + (`opengate/exception.py`, `fatal`), with messages that name the offending parameter. +- Engines construct the Geant4 objects during initialization; they are not meant for + user interaction and should not hold user-facing parameters. +- If you add a new engine step, wire it into the ordered initialization in + `opengate/engines.py` and make sure subprocess/threading paths still work + (`-p mp` in the test runner exercises them). + +## 6. Physics + +Physics lists and cuts live in `opengate/physics.py` and the C++ side. When adding a +physics list: register it on both sides, expose its name for the user, and add a test +asserting that the expected processes/particles exist rather than only that the +simulation runs (see `docs/source/developer_guide/developer_guide_physics.rst`). +Do not invent Geant4 class names — grep `core/` for the existing wrapper. + +## 7. Serialisation, multiprocessing, subprocesses + +- Anything added to a Gate object becomes part of its serialised state; keep it + JSON-friendly (see `opengate/jsonpickle` usage and `opengate/serialization.py`). +- Non-serialisable runtime state (Geant4 pointers) must be recreated in `__initcpp__`. +- Always run at least one test in `-p mp` mode when touching objects or engines. + +## 8. Checklist for a new object type + +- [ ] Python base class first, C++ base second; explicit super-calls, no `super()`. +- [ ] `__initcpp__()` used for the C++ constructor and `AddActions(...)`. +- [ ] `initialize()` calls `InitializeUserInfo` and `InitializeCpp` in the right order. +- [ ] Registered in the right manager; user-facing name exposed. +- [ ] User input validated in the manager, not the engine. +- [ ] Serialisation round-trip works under `opengate_tests -p mp`. +- [ ] Test added under `opengate/tests/src//testNNN_*.py`. +- [ ] User + developer doc page updated (`skills/documentation`). +- [ ] `pre-commit` clean (`skills/code-style`). \ No newline at end of file diff --git a/skills/code-style/SKILL.md b/skills/code-style/SKILL.md new file mode 100644 index 000000000..ed6878bbd --- /dev/null +++ b/skills/code-style/SKILL.md @@ -0,0 +1,88 @@ +# Skill: code-style + +**Goal:** make your change pass `pre-commit` and the reviewers on the first try. + +Formatting is **enforced by CI** via pre-commit.ci (see the badge in `README.md`): if you +push unformatted code, the bot will amend your PR with a formatting commit, or CI will +show a diff. Just run the hooks locally. + +--- + +## 1. Setup + +```bash +python -m pip install pre-commit +cd +pre-commit install # installs the git hook +pre-commit run --all-files # format everything now +pre-commit run # only staged files (also runs on commit) +``` + +## 2. What the hooks are (`.pre-commit-config.yaml`) + +| Hook | Rev | Applies to | +| --- | --- | --- | +| `trailing-whitespace` | v6.0.0 | everything | +| `black` | 26.5.1 | `*.py` | +| `clang-format` | v23.1.1 | `*.cxx`, `*.h`, `*.cpp`, … | + +There is **no** import sorter, linter or type checker configured. Do not "helpfully" +reformat unrelated files — a PR should diff only what it changes. + +## 3. Python conventions used in this repository + +Read `opengate/actors/`, `opengate/managers.py` and `opengate/engines.py` before writing +new code; follow the local idiom rather than inventing one. + +- **Naming**: `snake_case` for functions/variables/modules, `CamelCase` for classes. + Public API objects are created via manager factories (e.g. `sim.add_actor(...)`), + not by calling constructors directly. +- **Attributes named for Geant4 objects** are prefixed `g4_` (e.g. `g4_region`, + `g4_volume`) — the convention comes from `DesignGuidelines.md` and is still in use. +- **Imports**: absolute imports of the package (`import opengate as gate`, + `from opengate.actors import ...`). Inside `opengate/tests/src/` tests, import helpers + as `from opengate.tests import utility`. +- **Heavy dependencies are lazy**: this repo delays importing matplotlib/pandas/torch + until needed (`LazyModuleLoader` in `opengate/utility.py`). If you add a heavy import + on a hot path, check this pattern first. +- **Docstrings**: short, describing the user-visible behaviour of the class/function; + parameters are described where they are non-obvious. No enforced docstring style. +- **Errors**: raise the project's own exceptions/`fatal` from `opengate/exception.py` + rather than bare `Exception`, so the message is coloured and consistent. +- **Logging**: use `gate.log` / the project logger and `opengate/logger.py` helpers + instead of `print` in library code. Tests may print freely. +- **Comments** explain *why*; keep them sparse and in English. + +## 4. C++ conventions (`core/`) + +- clang-format decides layout — no manual alignment. +- Files: headers `*.h`, sources `*.cxx`/`*.cpp`; Geant4-derived helpers keep the + `Gate*` naming used throughout `core/opengate_core`. +- Bindings live next to the source they expose; when you add a binding, update the + corresponding `.cpp` that pybind11 compiles and check that `import opengate_core` + still exposes the symbol. +- C++17 (`-DCMAKE_CXX_FLAGS=-std=c++17`). + +## 5. Documentation formatting + +- reStructuredText under `docs/source/**`; Sphinx directives, see + `skills/documentation/SKILL.md`. +- Markdown only at the repo root (`README.md`, `CHANGELOG.md`, `AGENTS.md`, skills). +- Trailing whitespace is stripped everywhere, including `.rst` — long code blocks with + significant indentation must use Sphinx literal blocks, not whitespace hacks. + +## 6. Before committing + +```bash +pre-commit run --files # or: pre-commit run --all-files +git status # no build/output artifacts staged +git diff --stat # only intended files touched +``` + +- [ ] Hooks pass locally. +- [ ] Nothing from `.gitignore`'s "DATA & OUTPUTS" list is staged + (`*.mhd`, `*.raw`, `*.root`, `*.npz`, `*.csv`, `output/`, `data/`, logs). +- [ ] New files in `skills/` or docs are referenced from the relevant index + (`AGENTS.md` skills table, `docs/source/*/index.rst`). +- [ ] Commit messages describe the *why*; several small commits beat one huge one + (`docs/source/developer_guide/developer_guide_contribute.rst`). \ No newline at end of file diff --git a/skills/documentation/SKILL.md b/skills/documentation/SKILL.md new file mode 100644 index 000000000..029e007ee --- /dev/null +++ b/skills/documentation/SKILL.md @@ -0,0 +1,116 @@ +# Skill: documentation + +**Goal:** add or update user/developer documentation and build the Sphinx site locally. + +Docs are reStructuredText under `docs/source/`, rendered by Sphinx with the PDJ theme and +published on Read the Docs (). + +CI ignores changes under `docs/**` (`paths-ignore` in `.github/workflows/main.yml`), so a +docs-only PR does not trigger the build — but a broken reference *will* break the Read the +Docs build. Build locally. + +--- + +## 1. Layout + +``` +docs/ +├── Makefile, make.bat # `make html` +├── requirements.txt # sphinx + theme + autoapi + copybutton +└── source/ + ├── conf.py # Sphinx config + ├── index.rst # top-level toctree + ├── user_guide/ + │ ├── index.rst + │ ├── user_guide_installation.rst + │ ├── user_guide_intro.rst, user_guide_first_simulation.rst + │ ├── user_guide_.rst (narrative pages) + │ └── user_guide_reference_.rst (exhaustive references) + └── developer_guide/ + ├── index.rst + ├── developer_guide_installation.rst + ├── developer_guide_contribute.rst + └── developer_guide_.rst +``` + +Convention: `user_guide_reference_*.rst` for exhaustive parameter references, +`user_guide_*.rst` for task-oriented pages. + +## 2. Building locally + +```bash +cd docs +python -m pip install -r requirements.txt # sphinx, theme, autoapi, copybutton, ... +make html # output in docs/build/html +``` + +Open `docs/build/html/index.html` in a browser. Treat warnings (unresolved references, +malformed directives, missing toctree entries) as errors — fix them. + +Faster iteration: `sphinx-autobuild` if available, or read the warnings as they appear +without opening the browser. + +## 3. Adding a page + +1. Create `docs/source//.rst` using a title + underline + of matching length, and a hierarchy of `=`, `-`, `~` for sections. +2. **Add it to the toctree** of the corresponding `index.rst` — a page not in a toctree + produces a warning and is invisible on Read the Docs. +3. Cross-reference with labels instead of hard-coded HTML links where possible. +4. Verify with `make html`. + +Typical page skeleton: + +```rst +My new feature +============== + +Short statement of what the feature does and who needs it. + +Usage +----- + +.. code:: python + + import opengate as gate + + sim = gate.Simulation() + ... + +Parameters +---------- + +Note: such and such is only available with X. +``` + +## 4. Writing rules + +- **Code blocks** use `.. code:: python` / `.. code:: bash` and must be runnable (or + clearly marked as a fragment). +- **Admonitions** for warnings, e.g. `.. warning::`, `.. note::`, `.. tip::` + (`developer_guide_installation.rst` uses them throughout — copy that style). +- **Relative paths** in examples should point to real files in the repo. +- **Do not duplicate** upstream Geant4 documentation: link to it. +- Keep parameter names exactly as they appear in the code; verify by grepping + `opengate/` before writing them down. A wrong parameter name in the docs is worse than + no doc. +- Sphinx pages for the Python API are generated by `sphinx-autoapi`; a docstring in the + source plus the page's `automodule`/`autoclass` directive is enough to surface it. + +## 5. Documentation for a new feature: where to write what + +| You changed | Update | +| --- | --- | +| A user-visible parameter or behaviour | the matching `user_guide_reference_*.rst` **and** the narrative page that explains the workflow. | +| A new actor/source | `user_guide_reference_actors.rst` / `user_guide_reference_sources.rst` + a short section in the corresponding `user_guide_*.rst`. | +| The developer install / build | `developer_guide_installation.rst` — and keep it in sync with `.github/workflows/` (Geant4/ITK versions!). | +| The contribution workflow | `developer_guide_contribute.rst`. | +| A new agent-facing convention | this `skills/` directory + the table in `AGENTS.md` (not the Sphinx site). | + +## 6. Checklist + +- [ ] Page uses the existing naming convention and is listed in its `index.rst` toctree. +- [ ] `cd docs && make html` completes without **new** warnings. +- [ ] Every parameter name in the text was checked against the code. +- [ ] Versions quoted (Geant4, ITK, Python) match `.github/workflows/main.yml`. +- [ ] No build output committed (`docs/build/` is generated). \ No newline at end of file diff --git a/skills/engineering/SKILL.md b/skills/engineering/SKILL.md new file mode 100644 index 000000000..9febedebb --- /dev/null +++ b/skills/engineering/SKILL.md @@ -0,0 +1,108 @@ +# Skill: engineering + +**Goal:** pick the right *engineering role* for the work in front of you, then read that role's +playbook. + +The files in this directory are **role skills**: they answer "how should I think about this?" +rather than "what command do I run?". The task-oriented skills live one level up: +[`../environment-setup/SKILL.md`](../environment-setup/SKILL.md) (the dev environment, with +[`geant4-itk.md`](../environment-setup/geant4-itk.md) for compiling the native deps and +[`build-and-ci.md`](../environment-setup/build-and-ci.md) for packaging/CI), +[`../running-tests/SKILL.md`](../running-tests/SKILL.md), +[`../code-style/SKILL.md`](../code-style/SKILL.md), +[`../architecture/SKILL.md`](../architecture/SKILL.md), +[`../documentation/SKILL.md`](../documentation/SKILL.md). They are referenced *from* the role +skills, not duplicated by them. + +Start from [`../../AGENTS.md`](../../AGENTS.md) if you are new to the repository. The map of the +whole `skills/` tree is in [`../README.md`](../README.md). + +--- + +## 1. The roles in this directory + +| Role skill | Use it when | Owns | Does **not** cover | +| --- | --- | --- | --- | +| [`python-gate-developer.md`](python-gate-developer.md) | You write or refactor `opengate/**` Python code | Python API design, managers/actors/sources, the C++ boundary, serialisation | physics validity, test design | +| [`geant4-physics-expert.md`](geant4-physics-expert.md) | You choose a physics list / cut, or judge whether a result is physically plausible | Geant4 physics lists, EM models, cuts, statistics, realism | the GATE configuration layer, code structure | +| [`gate-physics-expert.md`](gate-physics-expert.md) | You configure the physics manager, **sources**, or **actors** | The GATE physics layer: physics manager, sources, actors, filters, biasing, normalisation | raw Geant4 model internals | +| [`gate-chemistry-expert.md`](gate-chemistry-expert.md) | You run or configure **Geant4-DNA chemistry** (chemistry list, chemistry world, scavengers, chemistry actors/counters) | The GATE chemistry layer: chemistry manager/list/world, track-structure EM regions, chemistry actors and counters | the general physics/source/actor machinery (see `gate-physics-expert`), raw Geant4-DNA model internals | +| [`geant4-geometry-expert.md`](geant4-geometry-expert.md) | You work at the raw G4 geometry layer, or add a geometry binding | G4 solids, logical/physical volumes, materials, navigation, overlaps, C++ bindings | the GATE user-facing volume API | +| [`gate-geometry-expert.md`](gate-geometry-expert.md) | You add or move volumes, solids, materials, regions or fields | The GATE geometry layer: volume tree, solids, materials, placement/repetition, regions | raw G4 navigation internals | +| [`simulation-debugger.md`](simulation-debugger.md) | A simulation crashes, hangs, or gives implausible output | Triage, reproduction, lifecycle-localised diagnosis | writing the fix's tests | +| [`test-writer.md`](test-writer.md) | You add or refactor a test | Test placement, assertions, tolerances, demonstrating failure | the code under test | + +## 2. The two layered pairs — do not read both by default + +The physics and geometry roles each come as a **pair**, deliberately split by **layer** so you +read one, not two: + +| Layer | Physics | Geometry | +| --- | --- | --- | +| **Geant4** (the model beneath GATE) | [`geant4-physics-expert.md`](geant4-physics-expert.md) | [`geant4-geometry-expert.md`](geant4-geometry-expert.md) | +| **GATE** (the API built on it) | [`gate-physics-expert.md`](gate-physics-expert.md) (+ [`gate-chemistry-expert.md`](gate-chemistry-expert.md) for Geant4-DNA chemistry) | [`gate-geometry-expert.md`](gate-geometry-expert.md) | + +- The **`geant4-*`** skills explain *why* the underlying engine behaves as it does — patch-level + physics, G4 object model, navigation, overlaps, the C++ layer in `core/`. +- The **`gate-*`** skills explain *how you configure and exploit it* — the managers, the volume + tree, **sources** and **actors**, regions, normalisation, filters. + +Each file states in its opening section when you are in *that* skill and not its sibling. When a +change spans both layers (a geometry change with a physics consequence, or a new binding), read +both — the GATE skill first, then the Geant4 one for the mechanism. + +## 3. How the roles compose + +A change is *designed* with the matching expert skill, *implemented* with the developer skill, +*proved* with the test-writer skill, and *triaged* with the debugger skill when it goes wrong. + +``` +design (physics / geometry expert) → implement (python-gate-developer) + → prove (test-writer) → triage (simulation-debugger) when it fails +``` + +Worked examples: + +| Task | Roles, in order | +| --- | --- | +| Add a new dose scorer | `gate-physics-expert` (what to score, normalisation) → `python-gate-developer` (the class) → `test-writer` | +| Move a detector / add a phantom | `gate-geometry-expert` (tree, materials) → `geant4-geometry-expert` if an overlap or binding is involved → `test-writer` | +| Change the physics list for a hadron therapy study | `geant4-physics-expert` (list choice, cuts) → `gate-physics-expert` (how to set it) → `test-writer` | +| Add or debug a Geant4-DNA chemistry run | `gate-chemistry-expert` (list, chemistry world, DNA EM region, counters) → `test-writer` | +| A simulation gives plausible-but-wrong numbers | `simulation-debugger` (classify) → the matching expert skill (is it physics, chemistry or geometry?) | + +## 4. Conventions every role skill follows + +1. **Environment first.** Each role opens by telling you to prove the environment is correct + (Geant4 version OK, no stale-`.so` warning) before drawing any conclusion. A physics or + geometry conclusion on a wrong build is worthless — see + [`../environment-setup/SKILL.md`](../environment-setup/SKILL.md). +2. **Machine-agnostic paths.** No role skill hard-codes an absolute path; per-machine values come + from `user_secrets.json` (`OPEN_GATE_REPO`, `OPEN_GATE_ENV`, `OPEN_GATE_DEPS`). +3. **Classify before you diagnose.** Every failure is a *repository bug* or an *environment issue* + ([`../status/found-bugs.md`](../status/found-bugs.md)). +4. **Evidence over claims.** A command actually run, with its output — not "should work". +5. **Cite real names.** The role skills name real classes and parameters + (`SolidBase`, `GenericSource`, `DoseActor`, `reference_physics_list_*`, …) taken from the + code; never invent a Geant4 class name — grep `core/` for the wrapper first. +6. **A checklist at the end.** Each role skill ends with a "definition of done"; use it before + reporting completion. + +## 5. Adding a role skill + +1. Create `skills/engineering/.md`, opening with `# Skill: engineering/` + and a one-paragraph `**Role:**` statement. +2. Say explicitly what the role **does not** cover, and link the neighbouring role it is often + confused with (the two layered pairs above are the model). +3. Follow the conventions in §4 — no absolute paths, cite real code names, checklist at the end. +4. **Add it to the tables in this file, in [`../README.md`](../README.md) and in the skills index + of [`../../AGENTS.md`](../../AGENTS.md)** — an undocumented skill is invisible. + +## 6. Checklist + +- [ ] You picked the role matching your task (§1), or the correct layer of a pair (§2). +- [ ] You read the role's **environment-first** section and confirmed the build is correct. +- [ ] The role's "Does not cover" column sent you to the right sibling skill if needed. +- [ ] You used the role's definition-of-done checklist before reporting completion. +- [ ] If a command in a role skill was wrong, you fixed the skill in the same change + (`../../AGENTS.md` §5). diff --git a/skills/engineering/gate-chemistry-expert.md b/skills/engineering/gate-chemistry-expert.md new file mode 100644 index 000000000..a9c2c049a --- /dev/null +++ b/skills/engineering/gate-chemistry-expert.md @@ -0,0 +1,268 @@ +# Skill: `engineering/gate-chemistry-expert` + +**Role:** configure and verify the *GATE 10 chemistry layer* — Geant4-DNA chemistry lists, the +track-structure EM region that chemistry requires, the chemistry world (background components and +scavenger reactions), and the chemistry actors and counters that score species and reactions. + +This is the chemistry sibling of [`gate-physics-expert.md`](gate-physics-expert.md). It owns the +GATE configuration layer for Geant4-DNA chemistry; read the physics skill for the general physics +list / source / actor machinery, and [`geant4-physics-expert.md`](geant4-physics-expert.md) for the +EM-model reasoning underneath. Read this skill whenever you touch `sim.chemistry_manager`, +`opengate/chemistry.py`, `opengate/actors/chemistryactors.py` or +`opengate/actors/chemistrycounters.py`. + +> **Alpha-stage feature.** The code itself says so (`ChemistryManager.alpha_warning_message` in +> `opengate/managers.py`, and the warning at the top of `docs/source/user_guide/user_guide_chemistry.rst`): +> interfaces, behavior and outputs may still change and must be validated before production use. +> Never present a chemistry result as validated, and expect the API to move. + +--- + +## 1. Establish the environment first + +Chemistry conclusions on a wrong build are worthless, and chemistry is more sensitive to a stale +build than most of GATE because it depends on Geant4-DNA chemistry lists and the +`GateVChemistryActor` / `GateTimeStepAction` C++ bridges being present in the rebuilt `.so`. + +```bash +cd "$OPEN_GATE_REPO" && source "$OPEN_GATE_ENV/bin/activate" +opengate_tests -t chemistry/test101_chemical_counting_actor.py +# the run header must print: Geant4 version is OK +``` + +If `Geant4 version is not ok` appears, or an import of a `G4EmDNAChemistry*` class fails with +`is not bound in opengate_core`, stop — that is a stale/incomplete build, not a chemistry problem +(`../environment-setup/SKILL.md` §3.3, `../environment-setup/geant4-itk.md` §7). A `Chemistry list '' is not bound in +opengate_core` fatal is raised by `ChemistryList.initialize_before_runmanager`, and it means the +same thing. + +## 2. Where chemistry lives in this repo + +| Concern | Location | +| --------------------------------------------------- | ------------------------------------------------------------------------------- | +| Chemistry list, world, species/reactions/dissociations | `opengate/chemistry.py` | +| Chemistry-aware actors + their outputs | `opengate/actors/chemistryactors.py` | +| Counters (built-in and configured) | `opengate/actors/chemistrycounters.py` | +| `ChemistryManager` (user-facing) | `opengate/managers.py` | +| Chemistry engine (lifecycle wiring) | `ChemistryEngine` in `opengate/engines.py` | +| Physics-list forwarding for chemistry | `opengate/physics.py` (`g4_augmented_physics_list`, `set_chemistry_list`) | +| C++ side | `core/opengate_core/opengate_lib/` (`GateVChemistryActor`, `GateChemicalCountingActor`, `GateTimeStepAction`, `GateITTrackingInteractivity`, `GateChemistryController`) | +| User docs | `docs/source/user_guide/user_guide_chemistry.rst` | +| Developer docs | `docs/source/developer_guide/developer_guide_chemistry_architecture.rst` | +| Tests | `opengate/tests/src/chemistry/` | + +**Read `user_info_defaults` of the object you configure** rather than guessing a parameter name — +it is the authoritative API and what gets serialised. The chemistry architecture is described at +length in `developer_guide_chemistry_architecture.rst`; read it before changing the wiring rather +than rediscovering the lifecycle from the code. + +## 3. The three ingredients chemistry always needs + +Chemistry is **not** enabled by one switch. A working chemistry simulation needs all three, and +omitting one gives a silently chemistry-free run: + +1. **A standard physics list** — `sim.physics_manager.physics_list_name` (e.g. + `"G4EmStandardPhysics"`). The chemistry list is attached to an *augmented* physics list + (`g4_augmented_physics_list`) so its `ConstructParticle`/`ConstructProcess` are forwarded; you do + not build that yourself. +2. **A Geant4 chemistry list** — `sim.chemistry_manager.chemistry_list_name`, one of + `known_g4_chemistry_list_names` in `opengate/chemistry.py`: `G4EmDNAChemistry`, + `G4EmDNAChemistry_option1`, `…_option2`, `…_option3`. `"default"` resolves to the manager's + default via a setter hook; left `None`, chemistry stays off unless an actor explicitly requests + it. **Read `known_g4_chemistry_list_names`; do not invent a name.** +3. **Track-structure EM physics in the region of interest** — chemistry needs Geant4-DNA + track-structure EM where the radicals are produced. Configure it per region/volume, not + globally: + - `volume.set_track_structure_em_physics("G4EmDNAPhysics_option2")` (the usual way), or + - `sim.physics_manager.set_track_structure_em_physics(volume_name, …)` / + `…_in_region(region_name, …)`, or + - let a chemistry actor request it (see §6). + + A **global** track-structure EM setting is refused: `PhysicsManager` tells you to configure it + "only per region" (`opengate/managers.py`). + +Two actors requesting **different** track-structure EM for the **same** volume is a `fatal` +(`PhysicsManager` conflict check), so make the requests agree. + +`sim.chemistry_manager.time_step_model` selects the Geant4 chemistry time-step model: +`"SBS"` (default), `"IRT"`, or `"IRT_syn"` — the allowed values are enforced by the parameter. +Chemistry regions are also where the `G4EmDNAPhysics*` track-structure cut/step behaviour lives, so +a chemistry result is meaningless if the region is wrong. + +## 4. Customising the chemistry list + +`sim.chemistry_manager.chemistry_list` is a `ChemistryList` that **wraps one built-in Geant4 +chemistry list and appends to it** (it registers itself as the active chemistry list, having +deregistered the built-in object used only as a callback provider). + +- **Species** — `chem_list.add_chemical_species(name=…, definition_name=…, molecular_mass=…, + charge=…, electronic_level=…, diffusion_radius=…, diffusion_coefficient=…)`, or pass a + `ChemicalSpecies`. `definition_name` defaults to `name`. A species that already exists in the + molecule table has its properties *updated* rather than duplicated. +- **Bimolecular reactions** — `chem_list.add_reaction(reactant_a=…, reactant_b=…, + rate_constant=…, products=[…], reaction_type=…)`. `rate_constant` is a genuine `float` in + Geant4-DNA units (**M⁻¹ s⁻¹**, i.e. `dm³ mol⁻¹ s⁻¹`); `reaction_type` is `0` (totally + diffusion-controlled) or `1` (partially diffusion-controlled). **`H2O` is the implicit solvent + and is silently *not* added as an explicit reaction product** — do not "fix" that by adding it + again. `reactant_a`/`reactant_b` are **sorted** internally, so the reaction key is + order-independent. +- **Unimolecular dissociations** — `chem_list.add_chemical_dissociation(parent=…, products=[…], + probability=…, energy=…)`, or a `ChemicalDissociation`. +- **Customisation without a base list is a `fatal`**: if you add species/reactions/dissociations + you must have resolved a `chemistry_list_name` first (`ChemistryList.initialize_before_runmanager`, + `has_customizations()`). + +Duplicates are `fatal` rather than ignored: adding the same species name, the same reaction key or +the same dissociation twice stops the run — which is what you want, but it means an inherited +default list plus your additions can collide. + +## 5. The chemistry world: background components and scavengers + +The chemistry world is an **optional** box that defines the chemical background and scavenger +reactions. It is separate from the geometry volume tree. + +- Create it with `sim.chemistry_manager.create_chemistry_world(...)`, passing **either** a + `volume=` **or** the pair `translation=`/`half_size=` — never both, never neither (both are + `fatal`). **Only one chemistry world per simulation**; replacing it requires setting + `chemistry_manager.chemistry_world = None` first. +- `set_volume()` currently accepts **only a `BoxVolume`**, with **no repetition** and an + **axis-aligned** (identity rotation) box attached **directly to the world** — anything else is a + `fatal` listing the offending volume. `half_size` is derived as `size / 2`. +- **Components** (the chemical background): `world.add_component(molecule_name, concentration)` + with a **positive** concentration in the appropriate concentration unit (mol/L is the convention + in Geant4-DNA). Duplicate species are `fatal`. +- **Scavenger reactions**: `world.add_scavenger_reaction(tracked_molecule=…, scavenger=…, + products=[…], rate_constant=…, reaction_type=…)`. The `scavenger` **must already be a defined + component**; the validation (`validate_scavenger_configuration`) otherwise stops the run with the + list of known components. `rate_constant` must be non-negative. +- **`pH`** — the stock `G4DNAScavengerMaterial` expects an **integer** pH; a non-integer value is a + `fatal` at material creation. Set it only if you need it. + +Ordering matters: the scavenger material is built **after** the chemistry list has populated the +molecule table, and `G4DNAScavengerProcess` caches the material in `BuildPhysicsTable()`. The +`ChemistryEngine` handles that ordering for you — do not try to create the scavenger material by +hand before Geant4 initializes. + +## 6. Chemistry actors and their configuration order + +`ChemistryActorBase` (`opengate/actors/chemistryactors.py`) is the Python base; chemistry +participation itself is implemented in the C++ `GateVChemistryActor`, so the Python class is mostly +configuration, validation and actor discovery. `ActorManager.has_chemistry_actors()` and +`ActorBase.is_chemistry_actor` are what the engines use to decide chemistry is active. + +The one concrete actor today is **`ChemicalCountingActor`**, a passive chemistry-scoring actor +"inspired by chem6". Its geometry-relevant parameters: + +| Parameter | Meaning | +| --- | --- | +| `attached_to` | the volume whose chemistry is scored (must see the tracks) | +| `track_structure_em_physics` | DNA EM for the attached region; **defaults to `"G4EmDNAPhysics_option2"`** and can be set directly on the actor, which then requests it for the region | +| `track_only_primary` / `primary_pdg_code` | chem6-like logic restricted to the primary (default PDG 11 = e⁻) | +| `energy_loss_min` / `energy_loss_max` | kill the primary / abort the event above an accumulated energy loss (negative disables) | +| `min_kinetic_energy` | kill the primary below this kinetic energy | +| `let_cutoff` | restricted-LET cutoff; secondary energies below it are folded into the event deposit | +| `times_to_record` | explicit chemistry times at which species numbers and G-values are recorded | +| `number_of_time_bins` | if `> 0` and `times_to_record` is empty, logarithmically spaced scoring times like chem6 | + +`ChemicalCountingActor` **requests DNA EM for the region it is attached to** (this is resolved before +Geant4 constructs the region; see `PhysicsManager`'s actor-request collection). If you also set +`track_structure_em_physics` on the volume, set the **same** value or the run is `fatal`. + +Results land in the `results` output (`ActorOutputChemicalCountingActor`, JSON): counters such as +`recorded_events`, `chemistry_starts`, `chemistry_stages`, `pre/post_time_step_calls`, +`reaction_count`, `killed_particles`, `aborted_events`, `total_energy_deposit`, +`accumulated_primary_energy_loss`, `mean/std_restricted_let`, plus `species` and `times_to_record`. +The chemistry output currently stores **merged data only** — asking for `which != "merged"` +produces a warning and is ignored. + +## 7. Counters — declarative, and the multi-threaded policy + +Counters are declared on the actor via `counter_config` and exposed as actor outputs +(`ActorOutputChemicalCounter`). `chemistry_counter_types` in `opengate/actors/chemistrycounters.py` +maps the names you can pass: + +- `BuiltinMoleculeCounter` — species counts, backed by `G4MoleculeCounter`. Key parameters: + `consider_molecules` (`"all"` or a list), `ignored_molecules`, `active_lower_bound` / + `active_upper_bound`, `time_comparer`, and the time-consistency checks. A molecule named in + **both** `consider_molecules` and `ignored_molecules` is a `fatal`, as is an unknown molecule name + (the message lists the known names). +- `BuiltinReactionCounter` — reaction counts, backed by `G4MoleculeReactionCounter`; same common + time/verbose options. +- `ConfiguredReactionCounter` — counts **specific** reaction signatures given as + `tracked_reactions` (`TrackedChemicalReaction` objects or dicts), optionally as a time series + (`record_time_series`). It requires at least one tracked reaction and an actor exposing + `RegisterConfiguredReactionCounter`. +- `ConfiguredSpeciesCounter` — counts **specific** species (`tracked_species`), requiring + `RegisterConfiguredSpeciesCounter`. + +`ChemicalCountingActor` deactivates `configured_reaction_counter` and `configured_species_counter` +by default and supports **at most one** molecule counter for its built-in species path (more is a +`fatal`). + +**Manager policy must not conflict.** `required_molecule_counter_manager_policy` (reset before +event/run, reset master with workers, accumulate into master) is merged across all chemistry actors; +requesting the same key with two different values is a `fatal`. `ChemicalCountingActor` fixes +`reset_counters_before_event = reset_counters_before_run = True` and +`accumulate_counter_into_master = False`. If you add a chemistry actor that wants a different +policy, expect a conflict. + +## 8. Confining chemistry and multi-threading + +- `sim.chemistry_manager.confine_chemistry_to_volume` — when set, a single global chemistry + controller (`GateChemistryController`) **kills chemistry tracks starting outside that volume + subtree**. It accepts `None`, a volume name, or a volume object (anything else is a `fatal`). Use + it to keep the chemistry stage local to the region you care about; without it chemistry runs + across the whole world and costs time for no scoring benefit. +- Chemistry callbacks fan out through `GateTimeStepAction` and `GateITTrackingInteractivity`, which + are registered with the chemistry actors by `ChemistryEngine.initialize_after_runmanager`. A + chemistry actor that is not in `actor_manager.sorted_actors` (or is added after initialization) + will not receive callbacks — add actors before `run()`. +- Chemistry plus multi-threading is the least-exercised combination; see + `chemistry/test102_chemical_counting_actor_aggregate_mt.py`. Compare against a + `number_of_threads = 1` run before trusting an MT count, and remember §9's statistics rules — + thread aggregation is exactly where counter-policy conflicts surface. + +## 9. Physics scrutiny for chemistry results + +All of [`gate-physics-expert.md`](gate-physics-expert.md) §7 (statistics) and §8 +(physics-sensitive checks) applies. Chemistry adds its own: + +- **G-values / species yields** are the physical anchor: compare the number of species produced per + unit deposited energy against published Geant4-DNA values for the same chemistry list and + time-step model, within the statistics of your run. +- **The chemistry list *and* the option number change the result.** `G4EmDNAChemistry_option1/2/3` + differ in their reactions and dissociations; state which one you used, and never compare results + across options as if they were the same model. +- **The time-step model matters**: `SBS`, `IRT` and `IRT_syn` give different kinetics. Report it, + and do not compare runs that use different models. +- **The scoring times matter**: species numbers depend on *when* they are sampled. Report + `times_to_record` / `number_of_time_bins`, and remember `ChemicalCountingActor` builds + logarithmically spaced times when you leave them implicit. +- **Chemistry only runs where track-structure EM is active.** A plausible-looking but empty + chemistry output usually means the region or the confinement volume is wrong, not that "there was + no chemistry". +- **Never compare chemistry counts on a difference below the statistical uncertainty**, and vary the + seed (`sim.random_seed`) — chemistry populations are stochastic. + +## 10. Definition of done + +- [ ] Environment proven correct: `Geant4 version is OK`, chemistry lists bound in `opengate_core`, + no stale-build warning. +- [ ] The alpha-stage status acknowledged in anything you report or document. +- [ ] All **three** ingredients present and stated: physics list, chemistry list name (exact + `G4EmDNAChemistry*` name), track-structure EM region and model name. +- [ ] A `time_step_model` chosen deliberately and reported (`SBS` / `IRT` / `IRT_syn`). +- [ ] Every custom species/reaction/dissociation has a rate constant with correct units and an + explicit `reaction_type`; `H2O` not re-added as an explicit product. +- [ ] If a chemistry world is used: creation mode (`volume` xor `translation`/`half_size`), every + scavenger listed as a component, integer `pH` if any. +- [ ] Actor `attached_to` sees the tracks; the actor's `track_structure_em_physics` agrees with any + volume/region setting (a conflict is a fatal). +- [ ] Counters intentional; the molecule-counter **manager policy** does not conflict with other + chemistry actors. +- [ ] `confine_chemistry_to_volume` set when chemistry is only wanted locally. +- [ ] Result reported with its chemistry list option, time-step model, scoring times, seed and + statistical uncertainty; G-values or species yields checked against expectation. +- [ ] Any test asserting on chemistry quantities has a justified tolerance + (see [`test-writer.md`](test-writer.md)); MT result cross-checked against a single-thread run. +- [ ] Chemistry behaviour is only ever described as alpha / needs validation. diff --git a/skills/engineering/gate-geometry-expert.md b/skills/engineering/gate-geometry-expert.md new file mode 100644 index 000000000..6f1e0caa7 --- /dev/null +++ b/skills/engineering/gate-geometry-expert.md @@ -0,0 +1,208 @@ +# Skill: `engineering/gate-geometry-expert` + +**Role:** build and verify the *geometry* of a GATE 10 simulation — the volume tree, the solids +that bound it, the materials it is made of, its placement/repetition, and the fields attached to +it — so that the modelled detector is both physically correct and numerically sound. + +This skill is the GATE-facing geometry role. It complements +[`geant4-physics-expert.md`](geant4-physics-expert.md) (what happens *inside* the volumes) and +[`python-gate-developer.md`](python-gate-developer.md) (how the classes are structured). Use it +whenever you create or change anything under `sim.volume_manager` / `opengate/geometry/`. + +--- + +## 1. Establish the environment first + +Same rule as every physics/geometry role: a conclusion drawn on a wrong or stale build is +worthless. + +```bash +cd "$OPEN_GATE_REPO" && source "$OPEN_GATE_ENV/bin/activate" +opengate_tests -t geometry/test001_g4geometry_geometrymapping.py # or any geometry test +# the run header must print: Geant4 version is OK +``` + +If it prints `Geant4 version is not ok`, or a `PhysicsListBuilder registry differs…` warning, +stop and fix the environment (`../environment-setup/SKILL.md` §3.3, +`../environment-setup/geant4-itk.md` §7) before touching the +geometry. + +## 2. Where geometry lives in this repo + +| Concern | Location | +| --------------------------------------------------- | ------------------------------------------------------------------------------------------ | +| Volume tree, placement, repetition, regions | `opengate/geometry/volumes.py` (`VolumeBase` and subclasses) | +| Solids (shapes) | `opengate/geometry/solids.py` (`SolidBase` + one class per shape) | +| Materials and HU/voxel material tables | `opengate/geometry/materials.py` | +| Electromagnetic fields attached to volumes | `opengate/geometry/fields.py` | +| Geometry helpers | `opengate/geometry/utility.py`, `volume_info.py` | +| User-facing manager / checks | `VolumeManager` in `opengate/managers.py` | +| C++ side (G4 helpers, unique volume IDs, voxelizer) | `core/opengate_core/opengate_lib/GateGeometry*, GateUniqueVolumeID*, GateVolumeVoxelizer*` | +| User docs | `docs/source/user_guide/user_guide_reference_volumes.rst`, `…_fields.rst` | +| Tests | `opengate/tests/src/geometry/` | + +**Read `user_info_defaults` of the class you are using rather than guessing a parameter name.** +Every geometry object declares its parameters there (name, doc, default, setter hook); that dict +is the authoritative API, and it is also what serialisation writes to JSON. + +## 3. The volume model — define the tree in your head before coding + +A GATE 10 geometry is a **tree of `VolumeBase` objects**, each with a mother. The pieces: + +- **World volume** — created automatically by `gate.Simulation()`; everything must descend from + it. A volume whose `mother` does not resolve is a fatal error, not a warning. +- **Solids** are *shapes*, not volumes: one `SolidBase` can back several volumes; a volume can + also be a boolean compound (`BooleanSolid`, built from `creator_volumes`). +- **Placement** is `translation` + `rotation` **relative to the mother**, each of which may be a + *list* — the list length is what **repeats** the volume (there is no separate `repeater` + parameter). Keep the two lists consistent in length; a mismatch silently misplaces daughters. +- **Regions** — named sets of volumes used by cuts, stepping and biasing. Cuts and production + thresholds are set per region, so how you group volumes into regions is a physics decision + (see [`geant4-physics-expert.md`](geant4-physics-expert.md) §4). + +When you add a volume, always write down: mother, material, solid, placement, and (if any) the +region it belongs to. A missing entry in that list is the most common geometry mistake. + +## 4. Choosing and defining a solid + +Shapes available in `opengate/geometry/solids.py` (grep the file — do not invent a class name): + +`BoxSolid`, `SphereSolid`, `EllipsoidSolid`, `TubsSolid`, `ConsSolid`, `PolyhedraSolid`, +`HexagonSolid`, `TrdSolid`, `TrapSolid`, `TesselatedSolid`, `ImageSolid`, `BooleanSolid`. + +Rules that pay off: + +- **Choose the simplest shape that respects the physics.** A box approximation of a curved + detector changes attenuation and scatter paths; it is a modelling decision, not a shortcut. +- **Use `TesselatedSolid` for STL/arbitrary surfaces** and `ImageSolid` for voxelised images — + both are much heavier than primitives, so reserve them for shapes you cannot express + analytically. Voxelised geometries also cost memory proportional to the image size. +- **Prefer a boolean compound over hand-placed subtraction volumes** when a shape is a + difference/intersection — `BooleanSolid` is built through the G4 boolean solids and keeps the + intent explicit. +- GATE 10 lengths/angles are numbers **with units** (`gate.g4_units.mm`, `.cm`, `.deg`, …). + Mixing a bare number where a length is expected is a silent scale error (§7). + +### 4.1 Boolean volumes — the operand must NOT be placed +`subtract_volumes(a, b)` / `unite_volumes` / `intersect_volumes` create a **new** volume from two +others. The operands keep their own `build_physical_volume` flag, so if you created them with +`sim.add_volume(...)` they are **also placed in the world** — and the result is an overlap: + +``` +G4Exception : GeomVol1002 +Overlap is detected for volume box:0 (G4Box) + apparently fully encapsulating volume hole:0 (G4Tubs) at the same level! +☠️ Fatal: Some volumes overlap with the volume "box". +``` + +**Fix — clear the flag on *both* operands before combining them:** + +```python +box = sim.add_volume("Box", "box") +box.material = "G4_W" +box.build_physical_volume = False # operand only: never placed on its own +hole = sim.add_volume("Tubs", "hole") +hole.rmax = 0.4 * mm +hole.build_physical_volume = False # operand only: never placed on its own +block = subtract_volumes(box, hole, new_name="block") # the real volume +block.mother = "world" +block.material = "G4_W" +``` + +The overlap is reported as a **warning** first (`G4ExceptionSeverity.JustWarning`) and only +aborts at the *overlap check* stage, so a casual run can look like it merely warned. + +### 4.2 Area-matched shapes: the premise is a formula, not an eyeball +When comparing two shapes “of equal area” (e.g. the upstream `Tubs`-vs-`Hexagon` report, #1107), +compute the area from the **same** parameter GATE uses, and state the formula: + +| Shape | GATE parameter | Area | +| --- | --- | --- | +| `Tubs` | `rmax` (with `rmin=0`) | `pi * rmax**2` | +| `Hexagon` | `radius` = **circumradius** | `(3*sqrt(3)/2) * radius**2` = `(sqrt(3)/2) * a**2`, `a` = across-flats = `sqrt(3)*radius` | + +Getting `radius` wrong (circumradius vs. across-flats) changes the area by **3×**, which produces a +large fake “shape effect”. For the #1107 target area `0.5261 mm^2`: `Tubs` `rmax = 0.40922 mm`, +`Hexagon` circumradius `0.45 mm` (across-flats `0.77942 mm`). + +Before concluding that a *shape* changes transmission, verify the geometry premise independently +(pure-Python ray tracing of the same 2D footprints is enough — that is what the reporter did). + +## 5. Materials + +- Material names are **Geant4 NIST names** (`"G4_WATER"`, `"G4_AIR"`, `"G4_LEAD"`, …) unless you + define your own. Confirm a NIST name exists before using it — grep `core/` / the docs rather + than recalling it. +- Custom materials and elements are declared through the material database in + `opengate/geometry/materials.py` and the manager; a volume referencing an undefined material + fails at initialization. +- For CT-like phantoms use the **HU / voxel material** machinery (`read_voxel_materials`, + `HU_read_materials_table`, `HU_read_density_table`, `HU_linear_interpolate_densities`): an + image gives a per-voxel material, and the HU↔density conversion is a *modelling choice* with + dosimetry consequences. State the table you used and the interpolation. +- Density and composition errors are invisible in a successful run. If a result looks off by a + constant factor, check the material density before re-checking the physics. + +## 6. Repetition, placement and the volume tree + +- **Repeated volumes** come from a *list* of translations and/or rotations. The count is the + number of placements; `number_of_repetitions` is derived, not set by hand. +- **The tree is updated lazily.** Mutating a volume's mother/placement after creation marks the + tree dirty (`_request_volume_tree_update`) — read `volume_manager` state rather than a stale + cached reference. Adding a child after reading `children_volumes` can read a pre-update tree. +- **Unique volume IDs** (`GateUniqueVolumeID`) identify a placement; actors that score per + volume use them. Two volumes with the same name in different mothers are distinct IDs — do not + assume the *name* is unique. +- Keep names meaningful: they appear in actor output, logs and error messages, and renaming one + breaks any test or script that addressed it. + +## 7. Geometry is a silent-failure risk — check these explicitly + +Geometry bugs rarely raise; they change the numbers. + +1. **Overlaps and "daughter outside mother".** Enable the Geant4 geometry checks and *read* the + output; do not silence overlaps. Overlaps change track lengths, energy deposition and + coincidence rates while the simulation still runs to completion. +2. **Units.** A length in the wrong unit (mm vs cm) scales the whole geometry. Sanity-check one + known dimension against its physical value. +3. **Placement sign / axis.** A misplaced detector sees the wrong solid angle. Verify the + expected count rate changes in the direction you predicted, not just "it changed". +4. **Mother/attachment mismatch.** A scorer or source attached to a volume that does not contain + the particles produces zero, not an error. +5. **Voxelised image alignment.** Index-to-world mapping (origin, spacing, orientation) is easy + to get subtly wrong; assert on a few known voxels. +6. **Boolean/compound correctness.** A compound with a rotated/translated creator volume builds + without error but is not the shape you drew. + +See §9 for how to turn the first ones into assertions instead of eyeballing. + +## 8. Geometry vs. physics vs. scoring — keep the boundary clear + +- A geometry change that alters attenuation/scatter/solid angle is a **physics** decision: state + the expected effect before looking at the output, and justify it against + [`geant4-physics-expert.md`](geant4-physics-expert.md). +- Cuts and regions are shared between geometry and physics: the region a volume belongs to + determines its production thresholds. Changing the grouping changes the physics. +- If a scorer reads zero, first confirm the *geometry* puts the volume where you think (a quick + visualisation or a bounding-box print), then look at the actor. + +## 9. Visualise rather than assume + +The fastest geometry check is to look at the model. GATE 10 can render the geometry +(`opengate_visu`, and volume `color`/`style` user parameters: `style` is `default`/`solid`/ +`wireframe`). Rendering requires Qt-enabled Geant4; if the environment was built without it, +that is an environment limitation, not a reason to skip the check — fall back to printing +bounding boxes (`SolidBase.bounding_limits`, `bounding_box_size`) and asserting on them. + +## 10. Definition of done + +- [ ] Environment proven correct (Geant4 version OK); no stale-`.so` warning. +- [ ] Mother, material, solid, placement and region stated for every volume you added. +- [ ] Solid is the simplest shape that respects the physics; heavy shapes justified. +- [ ] Units explicit on every length/angle; one known dimension sanity-checked. +- [ ] Tree state read after mutation, not from a stale cached reference. +- [ ] Geometry checks run and their output read — no ignored overlaps. +- [ ] Material names/densities verified (NIST name exists, or the custom definition is correct). +- [ ] Voxelised/boolean geometry asserted on known voxels/features, not just "it ran". +- [ ] Any resulting physics change justified and, if it is a test, tolerance explained + (see [`test-writer.md`](test-writer.md)). diff --git a/skills/engineering/gate-physics-expert.md b/skills/engineering/gate-physics-expert.md new file mode 100644 index 000000000..eca7ce9c6 --- /dev/null +++ b/skills/engineering/gate-physics-expert.md @@ -0,0 +1,209 @@ +# Skill: `engineering/gate-physics-expert` + +**Role:** decide and verify the *GATE 10 physics layer* — the physics manager, how **sources** +inject primaries, and how **actors** turn the resulting Geant4 tracks into scored quantities. + +[`geant4-physics-expert.md`](geant4-physics-expert.md) owns the *Geant4* physics (lists, EM +models, cross-sections, decay data). This skill owns the GATE layer that configures and exploits +them: `PhysicsManager`, `SourceManager`/sources, `ActorManager`/actors, filters and biasing. +Read the Geant4 skill for the underlying physics; read this one for the GATE machinery built +on top of it. + +--- + +## 1. Establish the environment before discussing physics + +Physics conclusions are worthless on a wrong build. Confirm **both**: + +```bash +cd "$OPEN_GATE_REPO" && source "$OPEN_GATE_ENV/bin/activate" +opengate_tests -t actors/test008_dose_actor.py # run header must print: Geant4 version is OK +python -c "from opengate_core import GateInfo; print(GateInfo.get_G4Version())" +``` + +GATE pins Geant4 `v11.4.2`. A `Geant4 version is not ok` line, or a +`PhysicsListBuilder registry differs from the linked Geant4` warning, means the environment — not +the physics — is wrong: fix it (`../environment-setup/SKILL.md` §3.3, +`../environment-setup/geant4-itk.md` §7) before interpreting +any result. + +## 2. Where the GATE physics layer lives + +| Concern | Location | +| ---------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------- | +| Physics lists, EM extensions, cuts, `PhysicsManager` | `opengate/physics.py` + `PhysicsManager` in `opengate/managers.py` | +| Sources (primaries) | `opengate/sources/**` (`generic.py`, `phspsources.py`, `voxelsources.py`, `gansources.py`, `beamsources.py`, `phidsources.py`, `lastvertexsources.py`) | +| Actors (scoring) | `opengate/actors/**` (`doseactors.py`, `digitizers.py`, `phspactors.py`, `arbitraryactors.py`, `biasingactors.py`, `coincidences.py`, `chemistryactors.py`) | +| Filters (track-level selection) | `opengate/actors/filters.py` | +| Actor output plumbing | `opengate/actors/actoroutput.py`, `dataitems.py` | +| C++ side | `core/opengate_core/opengate_lib/` (`GateSourceManager`, physics builder, `Gate…Actor` classes) | +| User docs | `docs/source/user_guide/user_guide_physics.rst`, `…_reference_actors.rst`, `…_reference_sources*.rst`, `…_reference_filters.rst` | +| Tests | `opengate/tests/src/physics/`, `…/source/`, `…/actors/`, `…/chemistry/` | + +**Read `user_info_defaults` of the object you are using** rather than guessing a parameter name — +that dict is the authoritative API and what gets serialised. + +## 3. Physics lists — the reasoning you must write down + +`opengate/physics.py` builds the available list names from +`reference_physics_list_base_class_names` + `reference_physics_list_em_extensions` + +`reference_physics_list_special_builders` (`_build_available_reference_physics_list_names`). +**Read that function**; never guess a name. + +For any change or recommendation, state: + +1. **The particles and energies involved** — this drives the valid list. +2. **The required EM accuracy** (e.g. low-energy EM models for < ~1 keV photons/electrons in + hadron therapy or microdosimetry). +3. **The list you picked and the list you rejected**, with the reason. +4. **The cost** — accuracy vs. runtime, and whether the change invalidates prior results. + +Verify the list is *actually* in effect, not just that the parameter name was accepted: + +- check the resolved list in the run output / `opengate_info`; +- confirm the linked Geant4 provides it — the `PhysicsListBuilder registry differs…` warning means + a registered list has no C++ binding (a stale build, B-001). + +## 4. Sources — the GATE layer that injects primaries + +A source defines **what, where, when and how many** primaries enter the simulation. Getting the +source wrong is the most common reason a correct detector model gives wrong absolute numbers. + +- **Every source is a `SourceBase`** (`opengate/sources/base.py`) with common parameters: + `attached_to` (the volume it is attached to — `mother` is **deprecated**), `start_time` / + `end_time`, and the **mutually exclusive** `number_of_primaries` vs. `activity` + (`n` is deprecated → `number_of_primaries`). +- **Absolute normalisation**: a result is only comparable to literature if the source + normalisation is known — per history? per Bq? per primary? State it, and check that + `activity`/`number_of_primaries` (with `half_life` where relevant) give the history count you + expect. +- **Key source types** (grep `opengate/sources/` for the current set): + `GenericSource` (particle + energy + position + direction), `VoxelSource`, + `VoxelizedPromptGammaTLESource`, `PhaseSpaceSource`, `GANSource`/`GANPairsSource`, + `IonPencilBeamSource`, `TreatmentPlanPBSource`, `PhotonFromIonDecaySource`, `LastVertexSource`, + `DebugSource`. +- **The generic source has three validated blocks** — position, direction, energy: + - position types include `point`, `sphere`, `box`, `disc`, `cylinder`, `surface_sphere`; + - direction types include `iso`, `histogram`, `momentum`, `focused`, `beam2d`, `cos`; + - energy types include `mono`, `discrete`, `histogram`, `interpolated`. + These are enforced by `PositionValidator` / `DirectionValidator` / `EnergyValidator` in + `opengate/sources/generic.py` — read the validator to see exactly what is accepted rather than + guessing a keyword. +- **A source attached to the wrong volume produces zero primaries, not an error.** Attach to a + volume that actually contains the emission region. +- **TLE / prompt-gamma and phase-space sources** carry their own physics subtleties (rejection + sampling, weights); a weighted source changes how you must normalise the result. +- Randomness is controlled by `sim.random_seed` (`"auto"` or an integer); log the resolved + `current_random_seed` when reporting a result. + +### 4.1 Building a beam harness — five traps that cost real debugging time +Every item below was hit **in this repository** while reproducing upstream #1107. Each one +produced a *plausible-looking but meaningless* result rather than a crash, so none of them is +self-evident from the output. Build the harness with these in mind, and **sanity-check it +before you believe any number it prints**. + +| Trap | What happens | Fix | +| --- | --- | --- | +| `direction.type = "beam2d"` is **not** a parallel beam | Needs a Gaussian divergence `sigma` (`GateGenericSource.cpp`: `SetBeamSigmaInAngX/Y`). With `sigma=[0,0]` it recorded **1 of 50 000** primaries. | For a parallel beam along an axis use `direction.type = "momentum"` + `direction.momentum = [0,0,1]` (verified: **50 003 of 50 000** through a scorer in an air world). | +| `number_of_primaries` **and** `activity` together | `fatal`: `Cannot use both the two parameters 'number_of_primaries' and 'activity' at the same time.` (raised in the *subprocess*, so the parent only reports `The queue is empty…`). | Set exactly one. Prefer `number_of_primaries` for a controlled test. | +| Boolean operands are also **placed** | `G4PVPlacement::CheckOverlaps()` → `Overlap is detected for volume box:0 … fully encapsulating volume hole:0`, then `fatal: Some volumes overlap… Aborting.` | Set `operand.build_physical_volume = False` on **both** the raw base volume and the hole before `subtract_volumes(box, hole)`. | +| Scoring plane too thin / too far | The `PhaseSpaceActor` records nothing; the log says only `Empty output, no particles stored`. | Place the scorer immediately behind the block and make it thick enough to be crossed (`0.05 mm` beat `0.01 mm` here). | +| A **point** source in a large world | Almost no primaries reach the scorer (`iso` from a point → **5 records / 50 000**) because of solid angle. | Use a `box`-position beam whose footprint covers the feature under test. | + +Two further rules that follow from the above: + +- **`start_new_process=True` needs a `if __name__ == "__main__":` guard** on macOS (spawn). + Without it the child re-imports the module and dies with + `RuntimeError: An attempt has been made to start a new process before the current process has finished its bootstrapping phase.` +- **Validate the harness against a known answer first.** Before comparing `Tubs` to `Hexagon`, + run the same beam through an *empty* world and assert ~100 % transmission. A harness that + transmits 0 % through air cannot measure a 25 % difference. + +### 4.2 Instantiating a physics list outside a simulation aborts +`create_reference_physics_list_class("X")(0)` calls `SetParticleCuts` with no region set up: + +``` +G4VUserPhysicsList::SetParticleCuts : No Default Region +*** G4Exception : Run0254 *** Fatal Exception *** Aborting execution *** +``` + +This is **not** a defect in the list — a physics list can only be instantiated inside a running +simulation. To check a list name is constructible, build the *class* and run a minimal +simulation (`Simulation` + world + source + `sim.run()`), as in +`opengate/tests/src/physics/`. + +## 5. Actors — the GATE layer that turns tracks into numbers + +Actors attach scoring to volumes. Two families: + +- **Dose / deposit actors** (`opengate/actors/doseactors.py`): `DoseActor`, `TLEDoseActor`, + `LETActor`, `BeamQualityActor`, `REActor`, `RBEActor`, `ProductionAndStoppingActor`, + `FluenceActor`, `EmCalculatorActor`. These score per voxel and write images; the + voxelisation, `size`/`spacing`/`origin`, and the scorer's volume are the things that go wrong. +- **Digitizers / coincidence / phase space** (`opengate/actors/digitizers.py`): + `DigitizerAdderActor`, `DigitizerReadoutActor`, `DigitizerBlurringActor`, + `DigitizerSpatialBlurringActor`, `DigitizerEfficiencyActor`, `DigitizerEnergyWindowsActor`, + `DigitizerDeadTimeActor`, `DigitizerPileupActor`, `DigitizerHitsCollectionActor`, + `DigitizerProjectionActor`, `CoincidenceSorterActor`, `PhaseSpaceActor`. + +Rules that prevent most actor mistakes: + +- **Score into the volume that sees the particles.** An actor attached to a volume the tracks do + not enter writes zeros, silently. +- **Filters and attributes select *what* is scored**: `opengate/actors/filters.py` (particle, + energy, process, volume, …) and the `*InVolumeAttribute` classes let you restrict scoring to + specific tracks/steps. A missing filter changes the quantity you think you measured. +- **Know the units and the normalisation of every actor output** before comparing to literature — + dose per history vs. per Bq, energy vs. dose, per-voxel vs. total. +- **Digitizers are for detector response**: blurring, dead time, efficiency and energy windows + model the readout. Dead time and pile-up are *rate-dependent* — their effect changes with + activity, so test them at the rates you care about. +- **Never invent a binding.** A `Gate…Actor` class must exist in `core/` **and** in the rebuilt + `.so`; a missing one means a stale build (`../environment-setup/geant4-itk.md` §7). + +## 6. Cuts, regions and biasing + +- Cuts are **range-based, per region**, expressed in the simulation's units; a global cut plus + per-volume overrides is normal. A wrong cut is a **silent** physics error — the run succeeds and + the numbers look plausible. +- When you change a cut, **re-derive the expected effect** (e.g. dose in a thin layer) *before* + looking at the output, so you can distinguish "changed as expected" from "changed otherwise". +- Assert on the cut Geant4 actually adopted, not the value you set. +- Biasing (`opengate/actors/biasingactors.py`, `GateAcceptanceAngle*`) changes the sampling: a + biased run's **weights** must be applied to get a physical result. Un-weighted biased output is + wrong by construction. + +## 7. Statistics — the most common physics mistake + +Simulation output is stochastic; a number without an uncertainty is not a result. + +- Never compare two runs on a difference smaller than their combined statistical uncertainty. +- Justify tolerances from the **statistics**, not to make a test pass: the relative sigma of a + scored quantity scales roughly as `1/sqrt(N)` histories. +- **Vary the seed** before believing a difference; a change that reproduces with one seed only is + noise. +- Prefer scoring more histories once over repeating a low-statistics run many times. + +## 8. Physics-sensitive things worth checking explicitly + +- **Decay / half-life / branching ratios** — a wrong ion or decay setting gives a plausible but + wrong spectrum. +- **Energy-spectrum endpoints** — compare against the analytically expected endpoint. +- **Attenuation** — a mono-energetic beam through a known thickness should follow the expected + exponential within statistics. +- **Absolute normalisation** — units, per-what normalisation, and source activity vs. histories. +- **Coincidence / timing** — time windows, dead time and sorter settings change the coincidence + rate; check the count against the expected geometric/activity scaling. + +## 9. Definition of done + +- [ ] Environment proven correct; linked Geant4 matches the pin; no registry warning. +- [ ] Physics list justified in writing (chosen **and** rejected alternatives). +- [ ] Source: type, attachment volume, position/direction/energy, and **normalisation** stated. +- [ ] Actor: attached to the right volume, filters/attributes intentional, output units and + normalisation stated. +- [ ] Cuts re-derived and asserted on the value Geant4 adopted; biasing weights accounted for. +- [ ] Result reported with its statistical uncertainty and the resolved seed. +- [ ] Conclusion replicated with at least one other seed when it rests on a small difference. +- [ ] Any test asserting on physics quantities has a justified tolerance + (see [`test-writer.md`](test-writer.md)). diff --git a/skills/engineering/geant4-geometry-expert.md b/skills/engineering/geant4-geometry-expert.md new file mode 100644 index 000000000..cef572f87 --- /dev/null +++ b/skills/engineering/geant4-geometry-expert.md @@ -0,0 +1,166 @@ +# Skill: `engineering/geant4-geometry-expert` + +**Role:** work at the *raw Geant4 geometry* layer — G4 solids, logical/physical volumes, +placements, materials, navigation, overlaps, and the C++ side of geometry in `core/`. Use this +when the GATE-level API in [`gate-geometry-expert.md`](gate-geometry-expert.md) is not enough, or +when you must understand what GATE is really building. + +Read [`gate-geometry-expert.md`](gate-geometry-expert.md) first if you only need the user-facing +volume API. This skill is the layer *below* it: the G4 concepts, the C++ bindings, and the +engineer-level reasoning about geometry soundness. + +--- + +## 1. When you are in this skill and not the GATE one + +| Symptom / need | Skill | +| ----------------------------------------------------------------------- | ----------------------------------------------------------------------- | +| Add or move a volume with `sim.volume_manager` | [`gate-geometry-expert.md`](gate-geometry-expert.md) | +| Add a new solid/shape type, or a new geometry binding | **this skill** + [`../architecture/SKILL.md`](../architecture/SKILL.md) | +| A G4 overlap / navigation error with a G4 class name in the message | **this skill** | +| Explain *why* GATE's repeated-volume or region model behaves as it does | **this skill** | +| Change material handling in `core/` | **this skill** | +| A physics effect you cannot explain from the geometry you wrote | both, in this order | + +## 2. Establish the environment before discussing G4 behaviour + +Raw-Geant4 reasoning is only valid on the pinned build. Confirm **both**: + +```bash +cd "$OPEN_GATE_REPO" && source "$OPEN_GATE_ENV/bin/activate" +python -c "from opengate_core import GateInfo; print(GateInfo.get_G4Version())" +opengate_tests -t geometry/test001_g4geometry_geometrymapping.py # header: 'Geant4 version is OK' +``` + +GATE 10 pins Geant4 `v11.4.2`. Navigation, overlap detection and solid-vs-solid behaviour differ +between patch levels; do not reason about a G4 class's behaviour on a version you have not +verified you are linked against (`../environment-setup/SKILL.md` §3.3). + +## 3. The G4 geometry object model (and where GATE maps onto it) + +Geant4 geometry is a strict hierarchy. GATE's `VolumeBase` produces exactly these G4 objects: + +| Geant4 concept | GATE object | Where | +| ----------------------------------------------------------------- | ---------------------------------------------------------------- | ------------------------------- | +| `G4VSolid` (a shape, no placement) | `SolidBase` subclasses | `opengate/geometry/solids.py` | +| `G4LogicalVolume` (shape + material + daughters) | the logical volume of a `VolumeBase` | `opengate/geometry/volumes.py` | +| `G4VPhysicalVolume` (a placement of a logical volume in a mother) | the physical volume of a `VolumeBase` (via `g4_physical_volume`) | `opengate/geometry/volumes.py` | +| `G4Region` (set of logical volumes with shared production cuts) | a GATE region | `VolumeManager` | +| `G4Field` / `G4FieldManager` | a field attached by name (`field` user parameter) | `opengate/geometry/fields.py` | +| `G4VUserDetectorConstruction` | built by GATE's engine, not by the user | `opengate/engines.py` + `core/` | + +Key consequences you must internalise: + +- **A solid is shared; a logical volume owns a material and its daughters; a physical volume is a + placement.** Two repeated placements of the same detector share the logical volume but have + distinct physical volumes and distinct unique IDs. +- **The world volume is special**: the top `G4PVPlacement`, the one with no mother, whose logical + volume must contain everything. GATE creates it in `gate.Simulation()`. +- **Naming is not identity.** In G4, names are labels; identity is by pointer/ID. Two logical + volumes with the same name in different subtrees are different objects — score by unique ID + (`GateUniqueVolumeID`), never by name. + +## 4. The C++ side in this repo + +Geometry-relevant C++ lives under `core/opengate_core/opengate_lib/`: + +| File(s) | Purpose | +| ------------------------------------------------- | ----------------------------------------------------------------------- | +| `GateGeometryUtils`, `GateHelpersGeometry` | GATE-level geometry helpers exposed to Python | +| `GateUniqueVolumeID`, `GateUniqueVolumeIDManager` | stable identity for a placement | +| `GateVolumeVoxelizer` | voxelise a volume (used by imaging/dose workflows) | +| `pyGate*.cpp` | the pybind11 wrapper registering each class in `core/opengate_core.cpp` | + +Rules: + +- **Never invent a G4 class name.** Grep `core/` and the Geant4 headers you are actually linked + against before writing one. +- Adding a new geometry binding means: implement the class, add a `py…` wrapper, **register it in + `core/opengate_core/opengate_core.cpp`**, then rebuild. An editable install does **not** pick up + C++ changes — the `.so` is stale until you rebuild, and the failure is a misleading + `AttributeError: module 'opengate_core' has no attribute 'Gate…'` + (`../environment-setup/geant4-itk.md` §7). +- Rebuild with the **incremental** path (`cmake` + `make` in `core/build/cmake.*`), parallelism via + `os.cpu_count()` / `OPEN_GATE_BUILD_JOBS` (B-007). + +## 5. Placement, rotation and repetition in G4 terms + +- A placement is a rotation **then** translation in the mother's frame; GATE's `rotation` + + `translation` user parameters map one-to-one onto the G4 transform. Rotation is a 3×3 matrix; + translation is a 3-vector. +- **Repetition** in GATE is a *list* of transforms on a single `VolumeBase`, expanded to N + physical volumes — it is not `G4Replica`/`G4Parameterised`. This matters when you need + parameterised behaviour: for those cases use GATE's own parametrisation/`TesselatedSolid` + paths, or the C++ building blocks, rather than expecting replica semantics. +- **The transform is relative to the mother**, so moving a mother moves its whole subtree. When a + daughter lands in the wrong place, walk the tree from the world down and check each transform, + not just the one you edited. + +## 6. Materials in G4 terms + +- GATE uses the **Geant4 NIST material database** for `G4_*` names; a custom material must be + defined (elements, isotope composition, density) before any volume references it, or + initialisation fails. +- Density is in G4 units internally; a wrong-by-10× density is a silent, systematic scaling of + every interaction length. It is the classic "everything is off by a constant" geometry cause. +- **HU / voxel materials** are a GATE layer over this: an image gives material indices, a table + maps HU→material and HU→density with an interpolation. The table and interpolation are + modelling choices with dosimetry impact — state them. +- Materials are shared objects: changing one changes every volume using it. Prefer defining a + distinct material over mutating a shared one. + +## 7. Overlaps, navigation and "daughter outside mother" + +These are the G4-level failures that bite hardest because they often do **not** stop the run. + +- **Overlap** — two physical volumes occupy the same space. GATE/Geant4 can detect this; read the + report. Rays through an overlap see duplicated or skipped material: dose, fluence and + coincidence counts shift plausibly but wrongly. +- **Daughter outside mother** — a daughter's extent exceeds the mother's; tracks may be lost at + the boundary and scoring volumes silently miss. +- **Tolerance** — G4 compares extents with a tolerance. A shape that only *just* fits may be + flagged or may not, depending on tolerance and patch level; do not design geometry that relies + on exact coincidence of surfaces. +- **Assert, do not eyeball.** Convert the checks you care about into explicit comparisons: walk + the tree, compare each daughter's bounding box against its mother's + (`SolidBase.bounding_limits` / `bounding_box_size`), and assert containment with the tolerance + you intend. + +## 8. Debugging raw-geometry problems + +1. **Classify first** — a G4 error message with a G4 class name is usually a real geometry + problem; a missing `Gate…` attribute is a *stale build* + ([`simulation-debugger.md`](simulation-debugger.md) §1). +2. **Reduce to the minimal tree** — world + your two volumes. If the overlap disappears, bisect by + adding volumes back. +3. **Print the transforms and bounding boxes** for the subtree in question; compare the world-space + extents by hand. +4. **Check units at the boundary** — a unit error is a *scale* error in the transform or the + solid's dimensions, and is invisible in a successful run. +5. **Check the region/cut assignment** at the same time: a geometry that is fine but assigned to + the wrong region gives wrong physics with no error. +6. **Vary one thing at a time** and predict the expected change before re-running, so "it changed" + becomes "it changed by the amount I predicted". + +## 9. Geometry-sensitive checks worth making explicit + +- **Containment**: every daughter inside its mother, with the tolerance you intend. +- **Non-overlap**: no two physical volumes share space (or, if they must, the reason is written). +- **Alignment**: voxelised/boundary-critical geometry asserted on a few known voxels or edges. +- **Consistency of repeated placements**: same length for `translation` and `rotation` lists. +- **Material**: the intended density/composition on at least one known volume. +- **Solid angle**: for a detector meant to see a source, the predicted fraction vs. the modelled + one — a strong end-to-end geometry check. + +## 10. Definition of done + +- [ ] Environment proven correct and linked Geant4 version confirmed (pinned `v11.4.2`). +- [ ] The G4 mapping stated: which solid, logical and physical volumes, and in which mother. +- [ ] No invented G4 class names — every one grepped in `core/` or the linked headers. +- [ ] If C++ changed: class registered in `opengate_core.cpp`, `.so` **rebuilt**, warning gone. +- [ ] Overlap / containment / tolerance checked explicitly, not assumed. +- [ ] Units and density sanity-checked on at least one known quantity. +- [ ] Repeated placements have consistent `translation`/`rotation` list lengths. +- [ ] Any geometry change with a physics consequence is justified against + [`gate-geometry-expert.md`](gate-geometry-expert.md) and + [`geant4-physics-expert.md`](geant4-physics-expert.md). diff --git a/skills/engineering/geant4-physics-expert.md b/skills/engineering/geant4-physics-expert.md new file mode 100644 index 000000000..0ba5b3b74 --- /dev/null +++ b/skills/engineering/geant4-physics-expert.md @@ -0,0 +1,102 @@ +# Skill: `engineering/geant4-physics-expert` + +**Role:** decide and verify the *physics* of a GATE 10 simulation — physics lists, production +cuts, EM options, decay/transport, and the statistical realism of the result. + +This skill is about physics correctness, not about code structure. For code, see +[`python-gate-developer.md`](python-gate-developer.md); for failing simulations, see +[`simulation-debugger.md`](simulation-debugger.md). + +--- + +## 1. Establish the environment before discussing physics + +Physics conclusions are worthless on a wrong build. Confirm **both**: + +```bash +cd "$OPEN_GATE_REPO" && source "$OPEN_GATE_ENV/bin/activate" +opengate_tests -l 2>&1 | head -5 # must print: Geant4 version is OK +python -c "from opengate_core import GateInfo; print(GateInfo.get_G4Version())" +``` + +The project pins Geant4 `v11.4.2`. *Do not* reason about cross-section or EM-model behaviour +on a different Geant4 patch level — that is exactly the mistake that made two geometry tests +look like bugs (B-004/B-009). + +## 2. Where physics lives in this repo + +| Concern | Location | +| --- | --- | +| Physics lists (names, modular/augmented classes) | `opengate/physics.py` | +| EM extensions / aliases | `reference_physics_list_em_extensions`, `…_aliases` in `opengate/physics.py` | +| C++ physics binding | `core/opengate_core/opengate_lib/` (physics builder) | +| User documentation | `docs/source/user_guide/` (physics pages) + the reference pages | +| Tests that assert on physics | `opengate/tests/src/physics/`, plus `source/` and `chemistry/` | + +`opengate/physics.py` builds its available-physics-list names from +`reference_physics_list_base_class_names` + EM suffixes + special builders +(`_build_available_reference_physics_list_names`). **Read that function** rather than guessing +a list name, and never invent a Geant4 class name — grep `core/` for the wrapper first. + +## 3. Choosing a physics list — the reasoning you must write down + +Physics-list choice is a modelling decision with dosimetry consequences. For any change or +recommendation, state: + +1. **The particles and energies involved** (this drives the valid list). +2. **The required EM accuracy** (e.g. low-energy EM models for < ~1 keV photons/electrons, + in hadron-therapy or microdosimetry contexts). +3. **The list you picked, and the list you rejected**, with the reason. +4. **What it costs** — accuracy vs. runtime, and whether the change invalidates prior results. + +Verify the list is actually in effect rather than trusting the parameter name: + +- check the resolved name in the simulation output / `opengate_info`; +- confirm the linked Geant4 provides it — the runner warns with + `PhysicsListBuilder registry differs from the linked Geant4` when a registered list has no + C++ binding (this warning appeared on a stale build: B-001). + +## 4. Production cuts and thresholds + +- Cuts are range-based, expressed in the simulation's units, and are applied per region — + a global cut plus per-volume overrides is normal. +- A cut that is wrong for a region is a silent physics error: the simulation runs and the + numbers look plausible. +- When you change a cut, re-derive the *expected* effect (e.g. dose in a thin layer) before + looking at the output, so you can tell "changed as expected" from "changed for another + reason". +- Assert on the cut that Geant4 actually adopted, not on the value you set. + +## 5. Statistics: the most common physics mistake + +Simulation output is stochastic. A result is only meaningful with its uncertainty. + +- Never compare two runs on a difference smaller than their statistical uncertainty. +- Justify tolerances from the *statistics*, not to make a test pass: for N histories the + relative sigma of a scored quantity scales roughly as `1/sqrt(N)`. +- Vary the seed before believing a difference. A change that only reproduces with one seed is + noise. +- Prefer scoring more histories once over repeating a low-statistics run many times. +- `sim.random_seed` accepts `"auto"` or an explicit integer; `current_random_seed` records the + concrete seed resolved for the run (`opengate/managers.py`). Log it when reporting a result + so the run is reproducible. + +## 6. Physics-sensitive things worth checking explicitly + +- **Decay / half-life and branching ratios** — a source with the wrong ion or decay settings + produces a plausible-looking but wrong spectrum. +- **Energy spectrum endpoints** — compare against the analytically expected endpoint. +- **Attenuation** — a mono-energetic beam through a known thickness should follow the + expected exponential to within the statistics. +- **Dose normalisation** — check the units and the normalisation (per history? per Bq? per + primary?) before comparing to literature values. + +## 7. Definition of done + +- [ ] Environment proven correct; Geant4 version matches the pin. +- [ ] Physics list justified in writing (chosen **and** rejected alternatives). +- [ ] Cuts re-derived, and asserted on the value Geant4 adopted. +- [ ] Result reported with its statistical uncertainty and the resolved seed. +- [ ] Conclusion replicated with at least one other seed when it rests on a small difference. +- [ ] Any test asserting on physics quantities has a justified tolerance + (see [`test-writer.md`](test-writer.md)). \ No newline at end of file diff --git a/skills/engineering/python-gate-developer.md b/skills/engineering/python-gate-developer.md new file mode 100644 index 000000000..08174ec39 --- /dev/null +++ b/skills/engineering/python-gate-developer.md @@ -0,0 +1,91 @@ +# Skill: `engineering/python-gate-developer` + +**Role:** write and modify GATE 10 Python API code (`opengate/**`) that fits the existing +design and survives review. + +Read first: [`../architecture/SKILL.md`](../architecture/SKILL.md) (object lifecycle), +[`../code-style/SKILL.md`](../code-style/SKILL.md) (formatting), and +[`../running-tests/SKILL.md`](../running-tests/SKILL.md) (how to prove it works). + +--- + +## 1. Find the right file before writing + +| You want to… | Start in | +| --- | --- | +| Add/change a user-facing object or its parameters | `opengate/managers.py` (managers own user input) | +| Add an actor | `opengate/actors/` (`doseactors.py`, `digitizers.py`, `phspactors.py`, `filters.py`, `biasingactors.py`, `arbitraryactors.py`, `coincidences.py`, `chemistryactors.py`) | +| Add a source | `opengate/sources/` (`generic.py`, `phspsources.py`, `gansources.py`, `voxelsources.py`, `beamsources.py`, `phidsources.py`) | +| Change geometry/solids/materials | `opengate/geometry/` | +| Change physics lists or cuts | `opengate/physics.py` (+ the C++ side in `core/`) | +| Change run/init orchestration | `opengate/engines.py` | +| Add a reusable helper | `opengate/utility.py` — but check it is not already there | + +Never invent a new pattern when the surrounding module already has one. Pick the closest +existing class and mirror it. + +## 2. Hard rules + +Full mechanics and rationale: [`../architecture/SKILL.md`](../architecture/SKILL.md) §3 +(actors), §7 (serialisation). These are the rules; that page shows the code. + +- **No Geant4 object is created during the user phase.** Managers validate; engines build. +- **`__init__` takes no mandatory argument but the name**; parameters are set afterwards. +- **Actors: Python base class first, then the C++ base**, and call superclasses explicitly + (`ActorBase.initialize(self)`) — `super()` cannot resolve the C++ base. +- **The C++ constructor goes in `__initcpp__()`**, never in `__init__()`; the multiprocessing + path relies on this (`__setstate__` in `opengate/actors/base.py`). +- **Registries are `{name: object}` dicts**, not lists. +- **Validate user input in the manager**, and raise the project's own exceptions from + `opengate/exception.py` (`fatal`) — message must name the offending parameter. +- **Heavy imports stay lazy.** matplotlib/pandas/torch are loaded on demand + (`LazyModuleLoader` in `opengate/utility.py`); do not add a top-level heavy import on a hot + path. +- **Library code logs, it does not print.** Use the project logger (`opengate/logger.py`), + not `print`. +- **Everything you add becomes serialised state.** Keep it JSON-friendly + (`opengate/serialization.py`); recreate runtime-only state in `__initcpp__`. + +Formatting of all of the above is [`../code-style/SKILL.md`](../code-style/SKILL.md)'s job — +run `pre-commit` rather than matching style by eye. + +## 3. Adding a user-facing parameter + +1. Declare it in the owning manager's `user_info` schema (name, type, doc, default) — see the + big `user_info` blocks in `opengate/managers.py`. +2. Attach a setter hook if the value needs validation or normalisation + (e.g. `_setter_hook_verbose_level`). +3. Consume it during initialization, in the engine or object `initialize()`. +4. Document it (see `../../documentation/SKILL.md`) — reference page **and** the narrative + page. +5. Add a test that *fails before your change*. + +Never remove or rename a public parameter silently; check +`docs/source/user_guide/user_guide_reference_*.rst` and add a deprecation path +(`opengate/base.py` has the helpers). + +## 4. Working with the C++ boundary + +If your Python change needs a new binding: + +- the C++ class lives under `core/opengate_core/opengate_lib/` and must be registered in + `core/opengate_core/opengate_core.cpp`; +- **the compiled `.so` only updates when you rebuild** — an editable install does *not* pick up + C++ changes, and the failure mode is a misleading + `AttributeError: module 'opengate_core' has no attribute 'Gate…'` + ([`../environment-setup/geant4-itk.md`](../environment-setup/geant4-itk.md) §7); +- rebuild via the **incremental** path (re-run `cmake` + `make` in `core/build/cmake.*`), not a + full reinstall; +- build parallelism is now `os.cpu_count()`, overridable with `OPEN_GATE_BUILD_JOBS` + (was hardcoded to 4 — B-007). + +## 5. Definition of done + +- [ ] Follows the closest existing module's structure; no new competing pattern. +- [ ] User input validated in the manager; project exceptions used. +- [ ] Serialisation round-trip works — verified with `opengate_tests … -p mp` (multiprocessing). +- [ ] A test exists that fails without the change and passes with it + (see [`test-writer.md`](test-writer.md)). +- [ ] Docs updated (reference + narrative). +- [ ] `pre-commit` clean. +- [ ] You ran the relevant tests yourself; no "should work" claims. \ No newline at end of file diff --git a/skills/engineering/simulation-debugger.md b/skills/engineering/simulation-debugger.md new file mode 100644 index 000000000..9d4d94a99 --- /dev/null +++ b/skills/engineering/simulation-debugger.md @@ -0,0 +1,158 @@ +# Skill: `engineering/simulation-debugger` + +**Role:** diagnose why a GATE 10 simulation crashes, hangs, produces nothing, or produces +implausible output — and report the *cause*, not the first symptom. + +**The single most important habit here: classify the failure before you investigate it.** +Most confusing failures in this repo are environment problems, not code bugs. Chasing them as +bugs wastes hours and can produce a wrong "fix". + +--- + +## 1. Triage: environment or code? (do this first, it is usually the answer) + +| Evidence | Verdict | Action | +| --- | --- | --- | +| `AttributeError: module 'opengate_core' has no attribute 'Gate…'` | stale compiled `opengate_core` | [geant4-itk](../environment-setup/geant4-itk.md) §7 | +| `Geant4 version is not ok` | wrong Geant4 | [environment-setup](../environment-setup/SKILL.md) §3.3 | +| Everything fails with `ModuleNotFoundError: 'opengate'` | venv not activated | [environment-setup](../environment-setup/SKILL.md) §3.1 | +| `Error while reading the file 'output_ref/…'` | test-data submodule behind the recorded commit | [environment-setup](../environment-setup/SKILL.md) §2.1 | +| `cannot allocate memory in static TLS block` | Geant4 TLS model | [geant4-itk](../environment-setup/geant4-itk.md) §6 | +| Import error for `torch`/`gaga_phsp`/`pytomography` | optional extras absent | [environment-setup](../environment-setup/SKILL.md) §5 | +| Reproducible from a clean env, on the pinned Geant4 | **code bug** | continue below | + +Run the §7 checklist of `environment-setup` and record the result. Only then treat it as a +repository bug — and say so explicitly in your report. + +## 2. Reproduce minimally and deterministically + +A bug you cannot reproduce on demand is not yet diagnosed. + +```bash +cd "$OPEN_GATE_REPO" && source "$OPEN_GATE_ENV/bin/activate" + +# serialize + deterministic + easy to read: +opengate_tests -t /testNNN_name.py -p sp -n 1 +``` + +- Always activate first — the runner spawns `python ` and an un-activated venv fails + every test opaquely (B-002). +- Read **`opengate/tests/log/.log`**: the full traceback, not the screen summary. +- Fix the seed when the failure is stochastic; vary it when the failure is intermittent. +- If the test grows, keep the reproduction in a scratch script outside the repo. + +## 3. Locate the failure in the simulation lifecycle + +GATE 10 runs in ordered phases; the phase tells you where to look. + +| Symptom | Likely phase | Look at | +| --- | --- | --- | +| Error while just creating objects (`gate.Simulation()`, `add_actor`) | user phase | the manager's input validation (`opengate/managers.py`) | +| `initialize()` raise | initialization | the object's `initialize()` / `InitializeCpp()` | +| Crash after "Starting simulation" | Geant4 run | geometry overlaps, physics list, C++ side | +| Runs, but the output is empty/zero | scoring | actor `output` config, filters, the scorer's volume | +| Hangs forever | `mp` deadlock / geometry | re-run `-p sp -n 1`; then look for geometry overlaps | +| Nondeterministic crashes under load | resource contention | `-n 4`, often a test/host issue, not code | + +## 4. Get more information + +- **Verbosity**: raise `sim.verbose_level` (user phase) and + `g4_verbose_level` / `g4_verbose_level_tracking` (Geant4 / tracking) — see the `user_info` + block in `opengate/managers.py`. +- **Multiprocessing**: re-run with `-p sp -n 1`. If it passes serially, the bug is in + serialisation or process handling, not physics. +- **Geometry**: overlaps and "daughter outside mother" are a classic source of empty or wrong + results; enable Geant4 geometry checks and read them, do not silence them. +- **Dump the state**: `sim.to_json_file(...)` / `json` output shows exactly what was passed; + diff it against what you intended. +- **Narrow the delta**: does it fail on a bare `gate.Simulation()` + your one object? If not, + bisect by removing objects until it stops failing. + +## 5. Common real causes (ranked by how often they bite) + +1. **Stale `opengate_core`** after pulling C++ changes (B-001) — looks like a code bug. +2. **Stale test-data submodule** (B-008/B-009) — looks like a physics/comparison failure. +3. **Your own reproduction harness**, not the repo (see §5.1) — the most under-diagnosed cause. +4. **Wrong physics list for the particles/energies** — see + [`geant4-physics-expert.md`](geant4-physics-expert.md). +5. **Wrong units.** GATE 10 accepts units as strings; a silently wrong unit scale produces + plausible, systematically wrong output. +6. **A `g4_` object not created**, or an `initialize()` that skipped `InitializeCpp`. +7. **Geometry overlap / missing mother volume.** +8. **Statistics too low**, or a tolerance that can never be met (see + [`test-writer.md`](test-writer.md)). +9. **Resource contention** when the full suite runs in parallel. + +### 5.1 “My harness is broken” — the third category, and the one that fools you +When you write a scratch script to reproduce an upstream issue, the failure modes of **the script** +look exactly like the bug you are hunting: zero counts, a crash, an “overlap”, a list that “does not +work”. Before you log anything as a repository bug, prove the harness is sound. + +Every item below was actually hit in this repository while triaging upstream issues — none of them +crashes in an obvious way. + +| What you see | What it really is | Fix | +| --- | --- | --- | +| `Exception: Cannot use both the two parameters 'number_of_primaries' and 'activity'` raised *in a child process*, so the parent only shows `The queue is empty. The spawned process probably died or crashed.` | **Two harness errors stacked**: an invalid source config, then a misleading parent-level message. | Set exactly one of the two. Read the **child** traceback (the parent's message is a symptom, not the cause). | +| `RuntimeError: An attempt has been made to start a new process before the current process has finished its bootstrapping phase.` | `sim.run(start_new_process=True)` without a `if __name__ == "__main__":` guard, on a **spawn** platform (macOS/Windows). | Wrap the entry point in `if __name__ == "__main__":`. | +| `G4Exception : Run0254 … G4VUserPhysicsList::SetParticleCuts : No Default Region … Aborting` | Instantiating a physics list **outside** a simulation (`create_reference_physics_list_class("X")(0)`). | Only instantiate inside a run; to check a name, run a minimal simulation. | +| `Overlap is detected … fully encapsulating volume …` then `Fatal: Some volumes overlap` | Boolean **operands were also placed** as normal volumes. | `operand.build_physical_volume = False` on both operands — see [`gate-geometry-expert.md`](gate-geometry-expert.md) §4.1. | +| `⚠️ Empty output, no particles stored in …phsp.root` | The scorer was never crossed (too thin, wrong place) or the source never pointed at it. | Sanity-check: does the same beam give ~100 % through an **empty** world? | +| A physics list “not working” | The harness set a **deprecated** parameter (`src.n`) or the wrong beam type. | Check the deprecation message; use `number_of_primaries`. | + +**The discipline that catches all of the above — validate against a known answer first:** + +1. Build the *simplest* version whose result you can predict (air world, no absorber). +2. Assert it gives that predicted answer (e.g. ~100 % transmission). +3. Only then add the feature under test (the hole, the phantom, the second thread). + +A harness that transmits 0 % through air cannot measure a 25 % difference. If you skip step 2, you +will spend the session measuring your own bug — and you may write it into `found-bugs.md` as a +repository defect. **Never log an upstream issue as “confirmed” from a harness that has not been +validated.** + +### 5.2 Read the issue's comments before you reproduce anything +Maintainers often post the root cause and the fixing commit in the thread. Re-deriving it is a +wasted session — and, worse, re-running it in the *wrong tree* produces a confidently wrong answer. + +**Before reproducing an upstream claim, check the comments for:** + +1. **A root-cause explanation** — if it exists, verify the claim in the code instead of re-running + a long simulation. (Both #1135 and #1143 were fully diagnosed by maintainers; the actionable + work was to confirm the mechanism and locate the commit, not to reproduce the symptom.) +2. **A fixing commit** — then run `git merge-base --is-ancestor HEAD` to see whether your + tree already contains it. +3. **`git tag --contains `** — **this is the one people skip.** + +**A claim about a *released* version cannot be tested on a tree that is ahead of that release.** +Real example from this repository: #1135 (“TLEDoseActor spikes under 8 threads”) was fixed on +`master` by `a92a62bf1`, but `git tag --contains a92a62bf1` returns **nothing** — the newest release +`10.1.1` does not contain it. So: + +- a user on the released wheel **still sees the bug** — their report is valid; +- running it in a `master` checkout looks **clean**, and concluding “not reproduced” would be + **wrong** — you would have dismissed a real defect. + +This is the mirror image of §5.1: there, the harness faked a bug; here, the *tree* hides one. In +both cases the fix is to check what you are actually measuring before you report a verdict. + +## 6. Reporting a failure you cannot fix + +Log it in [`../status/found-bugs.md`](../status/found-bugs.md) using the template, with the +**category** field filled in (`environment issue` vs `repository bug`). A report without: + +- exact commands, +- environment (commit, OS, Python, Geant4), +- the first error line, + +is not actionable. If it *is* an environment issue, fix the environment instead of the repo. + +## 7. Definition of done +- [ ] Failure classified: environment issue, repository bug, **or harness error** — stated explicitly. +- [ ] Minimal, deterministic reproduction (seed + exact command). +- [ ] The reproduction was validated against a case whose answer was known in advance (§5.1). +- [ ] Child-process tracebacks read, not just the parent's `queue is empty` symptom. +- [ ] Root cause identified, not just the first symptom. +- [ ] The cause is fixed where it actually belongs (environment, harness, or a code commit). +- [ ] The relevant test passes after the fix, and you ran it yourself. +- [ ] Findings recorded in `../status/found-bugs.md` if not fixed immediately. diff --git a/skills/engineering/test-writer.md b/skills/engineering/test-writer.md new file mode 100644 index 000000000..31d7135ec --- /dev/null +++ b/skills/engineering/test-writer.md @@ -0,0 +1,156 @@ +# Skill: `engineering/test-writer` + +**Role:** write tests for this repository that actually catch regressions — in the repo's own +runner, in the right place, with justified assertions. + +GATE 10 uses its own runner, **not pytest**. Read +[`../running-tests/SKILL.md`](../running-tests/SKILL.md) for the mechanics; this skill is about +test *design*. + +--- + +## 1. Placement and numbering + +- Location: `$OPEN_GATE_REPO/opengate/tests/src//`, where `` is one of + `actors/ advanced_tests/ chemistry/ external/ geometry/ misc/ physics/ source/`. +- Name: `testNNN_short_topic.py`, next free number in the whole suite: + + ```bash + cd "$OPEN_GATE_REPO" && source "$OPEN_GATE_ENV/bin/activate" + opengate_tests -l # highest test id (114 at the time of writing) + ``` + +- Helpers live in `opengate/tests/utility.py` — import as + `from opengate.tests import utility`. Check what already exists before writing new helpers; + `utility.test_ok(...)`, image comparison and file checks are already there. +- Reference data goes in the test-data submodule (`opengate/tests/data/`), never in the repo. + +## 2. Always demonstrate that the test can fail + +A test that cannot fail is worse than no test: it consumes suite time and gives false +confidence. Before committing: + +1. Run it with your fix → **pass**. +2. Revert your fix (or temporarily break the assertion) → **fail**. +3. Restore → **pass**. + +Record the command you used. If step 2 will not fail, the assertion is not testing what you +think. + +## 3. What to assert on + +Order of preference: + +1. **The user-visible outcome** (dose in a region, produced particles, phase-space content, + geometry parameters) — this is what a regression would break. +2. **A Geant4-level configured value** (set physics list, applied cut, volume placement) when + the outcome is too slow or too noisy to score. +3. **Never** on an internal variable or an implementation detail that may legitimately change. + +Assertions must survive legitimate refactoring. If your test breaks when someone renames a +private helper, it is over-specified. + +## 4. Tolerances — justify them, never tune them + +Stochastic assertions are where tests rot. + +- Derive the tolerance from the statistics: for `N` histories the relative sigma of a scored + quantity roughly scales as `1/sqrt(N)`. Choose `N` so the tolerance you need is comfortably + above the noise floor. +- State the statistics in the test (histories, seed) so the tolerance is reproducible. +- A tolerance loosened "until it passed" is a bug report, not a test. If you had to loosen it, + say so in the test docstring. +- Deterministic quantities (geometry, parameters, file contents) may use exact equality. + +## 5. Template + +```python +""" +Short description of the behaviour under test. + +Needs: (remove if none) +Statistics: (if the assertion is stochastic) +""" + +import opengate as gate +from opengate.tests import utility + + +def main(): + sim = gate.Simulation() + sim.random_seed = # fixed: make the test reproducible + # minimal configuration — only what this test is about + ... + + sim.run() + + # assert on the user-visible outcome, with a justified tolerance + value = + is_ok = abs(value - ) < + utility.test_ok(is_ok) + + +if __name__ == "__main__": + main() +``` + +## 6. Constraints the suite imposes + +- **Self-contained**: no network, no external data beyond `opengate/tests/data`. +- **Write only under `opengate/tests/output*/`** — these paths are git-ignored. A test writing + elsewhere pollutes the tree and risks being committed (this already happens once: a full run + leaves `simulation.json` in the repo root, B-006). +- **Keep runtime small** — seconds to a couple of minutes; the suite already runs 368 tests per + CI matrix entry. +- **No cross-test dependencies** unless you deliberately use the runner's dependency mechanism; + the runner splits mutually-dependent tests into a second round, which doubles complexity. +- **Declare optional dependencies** in the docstring; the suite skips such tests when the extra + is absent (e.g. 11 files are skipped for missing `torch`). + +## 7. When you write a *scratch reproduction* (not a test), the same rules apply +Triaging an upstream issue means writing a throwaway script. Treat it with the same suspicion as a +test, because a broken scratch script looks exactly like the bug you are hunting — and it is the +fastest way to log a **false** repository bug. + +- **Validate the harness against a known answer before using it.** Transmission through an *empty* + air world must be ~100 %; if your parallel beam gives 0 %, the bug is in your script. (Real + example: `direction.type = "beam2d"` with `sigma=[0,0]` is not a parallel beam — it recorded + 1 of 50 000 primaries. `momentum` + `momentum=[0,0,1]` recorded 50 003 of 50 000.) +- **Read the child process's traceback.** With `sim.run(start_new_process=True)` the parent often + reports only `The queue is empty. The spawned process probably died or crashed.`, while the real + cause (e.g. an invalid source config) is in the child's traceback. +- **A clean exit is not a result.** `⚠️ Empty output, no particles stored` means zero recorded — + that is a harness signal, not evidence about the code under test. +- **State the statistics** when you compare two configurations. A ratio computed from single-digit + counts (e.g. 2 vs 8 events) is noise; say so rather than reporting the ratio. +- Full catalogue of these traps: [`simulation-debugger.md`](simulation-debugger.md) §5.1 and + [`gate-physics-expert.md`](gate-physics-expert.md) §4.1. + +## 8. Verify, and be honest about what you did not verify + +```bash +source "$OPEN_GATE_ENV/bin/activate" +cd "$OPEN_GATE_REPO" +opengate_tests -t /testNNN_your_test.py # expect: 1/1 passed +opengate_tests -t /testNNN_your_test.py -p mp # must also work under multiprocessing +``` + +- Multiprocessing (`-p mp`, the default) exercises serialisation: **always run it**, because a + test that only passes serially is testing an unrealistic path. +- Read the failure in `opengate/tests/log/.log`, not the screen summary. +- If the test fails for environment reasons, fix the environment first + ([`../environment-setup/SKILL.md`](../environment-setup/SKILL.md) §2.1, §3.3, + [`../environment-setup/geant4-itk.md`](../environment-setup/geant4-itk.md) §7) — do not + weaken the assertion to accommodate a broken environment. + +## 9. Definition of done + +- [ ] Correct subdirectory, next free `testNNN_` number, snake_case topic. +- [ ] Demonstrated to fail without the change and pass with it. +- [ ] Asserts on a user-visible outcome; tolerance derived from the statistics and stated. +- [ ] Fixed seed; deterministic given that seed. +- [ ] Writes only to `opengate/tests/output*/`. +- [ ] Passes under `-p mp`. +- [ ] Runtime acceptable; optional deps documented. +- [ ] Any scratch reproduction was validated against a known-case answer before its result was used. +- [ ] The command and result quoted in your report — never "should pass". \ No newline at end of file diff --git a/skills/environment-setup/SKILL.md b/skills/environment-setup/SKILL.md new file mode 100644 index 000000000..e4c6003f1 --- /dev/null +++ b/skills/environment-setup/SKILL.md @@ -0,0 +1,658 @@ +# Skill: environment-setup + +**Goal:** obtain a virtual development environment in which the *whole* OpenGATE test +suite can run, prove it works before you change anything — and know how the project is +**built, packaged, released and tested in CI**. + +Do not skip the verification step. A tree that imports `opengate` but runs against a +stale compiled `opengate_core` produces "wrong but plausible" failures that waste hours. + +| Part | Sections | Use it when | +| --- | --- | --- | +| **Your environment** | §0–§3, §5, §7–§9 | you need a machine where the suite runs | +| **Build & CI deep-dives** | [`geant4-itk.md`](geant4-itk.md), [`build-and-ci.md`](build-and-ci.md) | you build Geant4/ITK/`opengate_core`, release a version, or debug a CI-only failure | + +This file is the **index** for the environment work: §0–§3 and §5 get you a working environment, +§7–§9 verify and tidy it. The two things that need real depth live in their own files — read them +only when the task calls for it: + +- **[`geant4-itk.md`](geant4-itk.md)** — compiling Geant4 and ITK and rebuilding the + `opengate_core` C++ extension (§4 below is a pointer to it). +- **[`build-and-ci.md`](build-and-ci.md)** — packaging, releasing, and how CI tests the wheels + (§6 below is a pointer to it). + +--- + +## 0. Two paths must come from the user (never invent them) + +This document is deliberately **machine-agnostic**: no absolute path, no username, no +drive letter. The per-machine values live in **`user_secrets.json` at the root of the +repository**, which is **git-ignored and never committed**. + +### 0.1 Read — or create — `user_secrets.json` + +**Always do this first.** The file is expected at `$OPEN_GATE_REPO/user_secrets.json`: + +```bash +cd "$OPEN_GATE_REPO" +cat user_secrets.json 2>/dev/null || echo "MISSING: user_secrets.json" +``` + +If it does **not** exist, **create it from the template below and then ask the user for the +real values** — placeholder values are not usable, and you must not guess a path. If it +exists but still contains `CHANGE_ME`, ask the user for the missing entries before +installing or building anything. + +The file is JSON with these keys: + +| Key | What it is | Rules | +| --- | --- | --- | +| `OPEN_GATE_REPO` | Absolute path of the OpenGATE working tree (the directory containing `VERSION`, `pyproject.toml`, `opengate/`, `core/`). | Must be a clone of this repository — often the current working directory. Must **not** be inside the environment. | +| `OPEN_GATE_ENV` | Absolute path of the Python environment used to build and test. | **Must be on non-volatile storage** — a real disk, not `/tmp`, `/run`, a tmpfs, an overlay, or a container's ephemeral layer, because a multi-minute C++ build and the downloaded Geant4 data (GBs) must survive a reboot/session. | +| `OPEN_GATE_DEPS` | Where Geant4 and ITK are built or installed, in `/geant4.11-build` and `/itk-build` subdirectories. | Only needed for the full C++ path ([`geant4-itk.md`](geant4-itk.md)). Also non-volatile: these are ~10 GB builds. | + +Template to create (the real file must contain the user's actual paths): + +```json +{ + "OPEN_GATE_REPO": "/absolute/path/to/this/opengate/clone", + "OPEN_GATE_ENV": "/absolute/non-volatile/path/to/the/venv", + "OPEN_GATE_DEPS": "/absolute/non-volatile/path/to/geant4-and-itk-builds" +} +``` + +**Never commit it** and never copy its contents into a committed file, a test, a doc page +or a commit message. `.gitignore` already excludes `user_secrets.json` and +`user_secrets.*.json`; verify with `git check-ignore -v user_secrets.json`. If it is ever +staged, remove it from the index (`git rm --cached user_secrets.json`) and tell the user. + +### 0.2 Load it into the shell session + +**Everything below uses these variables.** Export them from the file so no command ever +hard-codes a path: + +```bash +cd "$OPEN_GATE_REPO" # or: export OPEN_GATE_REPO= if you are not there yet +export OPEN_GATE_ENV="$(python3 -c 'import json;print(json.load(open("user_secrets.json"))["OPEN_GATE_ENV"])')" +export OPEN_GATE_DEPS="$(python3 -c 'import json;print(json.load(open("user_secrets.json")).get("OPEN_GATE_DEPS",""))')" +``` + +Verify the values make sense before using them — an empty, placeholder, or relative value +means the file is incomplete and you must go back to the user: + +```bash +echo "repo=$OPEN_GATE_REPO"; echo "env=$OPEN_GATE_ENV"; echo "deps=$OPEN_GATE_DEPS" +[ -d "$OPEN_GATE_REPO/.git" ] || echo "OPEN_GATE_REPO does not look like a git clone" +case "$OPEN_GATE_ENV" in ""|*CHANGE_ME*) echo "OPEN_GATE_ENV is not set by the user";; esac +df -h "$OPEN_GATE_ENV" 2>/dev/null # must show a real filesystem, not tmpfs +``` + +Windows PowerShell equivalent: + +```powershell +$s = Get-Content user_secrets.json | ConvertFrom-Json +$env:OPEN_GATE_REPO = $s.OPEN_GATE_REPO +$env:OPEN_GATE_ENV = $s.OPEN_GATE_ENV +$env:OPEN_GATE_DEPS = $s.OPEN_GATE_DEPS +``` + +### 0.3 Reuse an existing environment before creating one + +**Prefer reusing what is already on the machine.** A suitable environment may exist — +created by the user, by a previous session, or by `uv`. Detect it, validate it (§7), and +only build a new one if validation fails. + +Ask the user first, then look: + +```bash +# candidates the user may name, or that may already be set in the shell +env | grep -iE 'virtual_env|conda|open_gate' # already-active environment? +ls -d "$OPEN_GATE_REPO/.venv" 2>/dev/null # repo-local venv, if allowed +find "$HOME" -maxdepth 3 -name 'pyvenv.cfg' 2>/dev/null # venvs near the user's home +``` + +Identify what a candidate *is* before trusting it (run its interpreter directly): + +```bash +"$CANDIDATE/bin/python" -c 'import sys; print(sys.executable, sys.version)' +"$CANDIDATE/bin/python" -c 'import opengate, opengate_core; print(opengate.__file__); print(opengate_core.__file__)' +"$CANDIDATE/bin/uv" --version 2>/dev/null; "$CANDIDATE/bin/python" -m pip --version 2>/dev/null +``` + +Then answer three questions: + +1. **Is it non-volatile?** Reject a path under `/tmp`, `/var/tmp`, `/run`, `/dev/shm`, a + `tmpfs` mount, or inside the C++ build tree. Verify with `df -h "$CANDIDATE"`: if the + `Filesystem`/`Mounted on` points at `tmpfs`, or `df` reports a tiny size, reject it. +2. **Is it for *this* repo?** `opengate.__file__` must resolve inside `$OPEN_GATE_REPO` + (an editable install) — or the user must confirm a wheel install is intended. +3. **Is it consistent?** Both packages present, same version as `VERSION`, and the + compiled `opengate_core` newer than the C++ sources (§3.4, [`geant4-itk.md`](geant4-itk.md) §7). + +If it passes all three, set `OPEN_GATE_ENV` to it and skip to §5/§7. If it fails, tell the +user *which* check failed (and log it in `skills/status/found-bugs.md` if it looks like a +real defect) before rebuilding. + +### 0.4 Creating an environment when none is reusable + +Create it **at the user-provided path**, on non-volatile storage. Two supported tools: + +```bash +# with uv (preferred if available: no pip, fast resolution) +uv venv "$OPEN_GATE_ENV" + +# or with the stdlib +python3 -m venv "$OPEN_GATE_ENV" +``` + +If the user has not provided a path, propose one **outside** `/tmp` — for instance next to +the repository or under the user's home (`$HOME/pyenv/opengate`) — and **confirm it before +creating anything**. Never create an environment, a Geant4/ITK build, or the test-data +download inside a volatile directory. + +A `uv`-created environment has **no `pip`** (see §3.1); note which tool you used, since +every install command below has two forms. **Watch out**: the very first command that +references an install (`python -m pip ...`) will fail with `No module named pip` and exit +non-zero if you picked the wrong form — check the exit status, do not assume a silent +`&&`-chained script succeeded. + +**Never put build logs or build outputs under `/tmp`** (volatile): write long-running +build logs next to the environment or another user-provided non-volatile path. + +--- + +## 1. Decide which install you need + +| Situation | Install path | +| --- | --- | +| Only touching Python (`opengate/**`) | **Fast path** (§3): editable install, `opengate_core` from PyPI wheel or reused build. | +| Touching `core/**` (C++, bindings, Geant4/ITK behavior) | **Full path** ([`geant4-itk.md`](geant4-itk.md)): build Geant4, ITK and `opengate_core`. | +| Reproducing CI exactly / Qt visualization / debugging native crashes | **Full path** ([`geant4-itk.md`](geant4-itk.md)) + the CI-matching notes ([`build-and-ci.md`](build-and-ci.md) §5). | + +Rule of thumb: if `git diff --name-only master` shows anything under `core/`, you need +the full path. Otherwise the fast path is enough. + +## 2. Preconditions (always) + +Work in `$OPEN_GATE_REPO` (from `user_secrets.json`, §0.2). If the tree is not cloned yet, +clone it **to a non-volatile location the user agrees with** — the secrets file lives +*inside* the repo, so on a fresh clone you create it there and ask the user for the values: + +```bash +cd "$OPEN_GATE_REPO" # the existing clone + +# git-lfs is REQUIRED: opengate/tests/data is a binary submodule +git lfs install +git submodule update --init --recursive +``` + +If the submodule fetch fails behind a proxy/TLS interception: + +```bash +export GIT_SSL_NO_VERIFY=1 # CI itself does this (see .github/workflows/main.yml) +``` + +Check the data submodule is really populated before running anything: + +```bash +ls "$OPEN_GATE_REPO/opengate/tests/data" | head # must not be empty +ls "$OPEN_GATE_REPO/core/external/pybind11" | head # must not be empty +ls "$OPEN_GATE_REPO/core/external/fmt" | head # must not be empty +``` + +### 2.1 Check the submodule is at the *recorded* commit, not just populated + +A populated submodule can still be **behind** the commit this repo pins, in which case +reference data for some tests is missing and those tests fail with misleading comparison +errors while the test-data folder looks fine. This produced the only two failures in the +reference full-suite baseline (B-008/B-009). Compare the two hashes: + +```bash +cd "$OPEN_GATE_REPO" +echo "expected: $(git ls-tree HEAD opengate/tests/data | awk '{print $3}')" +echo "actual : $(git -C opengate/tests/data rev-parse HEAD)" +``` + +If they differ, fix it: + +```bash +git submodule update --init --recursive opengate/tests/data +``` + +`git status` will **not** warn you: `git` does not track empty directories, and +`utility.create_output_ref()` creates the reference folder with `exist_ok=True`, so a missing +dataset leaves an empty directory behind rather than a clear error. + +## 3. Fast path — Python development + + +```bash +python3 -m venv "$OPEN_GATE_ENV" # created in §0.4 if it did not exist +source "$OPEN_GATE_ENV/bin/activate" # Windows: "$OPEN_GATE_ENV/Scripts/activate" +python -m pip install --upgrade pip + +cd "$OPEN_GATE_REPO" +# installs opengate + pulls opengate-core==$(cat VERSION) from PyPI +python -m pip install -e . +``` + +The `opengate_core` wheel already contains Geant4 and ITK, so this is enough for almost +all Python-level work. If `$OPEN_GATE_ENV` was created with `uv`, use the `uv` form below. + +### 3.1 `uv`-managed environments + +A venv created by **`uv`** has **no `pip` module** — `python -m pip ...` fails with +`No module named pip`, and there is no `bin/pip` either. Use `uv pip` against the +environment instead: + +```bash +VIRTUAL_ENV="$OPEN_GATE_ENV" uv pip install -e . # from $OPEN_GATE_REPO +VIRTUAL_ENV="$OPEN_GATE_ENV" uv pip list | grep -i opengate +``` + +Or activate it and use `uv pip` without `VIRTUAL_ENV`: + +```bash +source "$OPEN_GATE_ENV/bin/activate" +uv pip install -e . +``` + +Detect which tool made an environment you are **reusing** (§0.3): + +```bash +"$OPEN_GATE_ENV/bin/python" -m pip --version # pip present -> pip form +ls "$OPEN_GATE_ENV/bin/pip" 2>/dev/null; grep -i uv "$OPEN_GATE_ENV/pyvenv.cfg" # uv form +``` + +**You must activate the environment (or export `PATH`), not just call its interpreter by +absolute path.** `opengate_tests` launches each test as the literal shell command +`python ` (`opengate/bin/opengate_tests_helpers.py`, `run_one_test_case`), so +an un-activated venv makes the subprocess resolve `python` to the system interpreter and +every test dies with `ModuleNotFoundError: No module named 'opengate'` — while the +runner itself imported `opengate` fine. Always `source "$OPEN_GATE_ENV/bin/activate"` +first. (Logged as B-002 in `skills/status/found-bugs.md`.) + +### 3.2 Smoke test + +First run downloads Geant4 data — allow several minutes. The download is triggered on +**first `import opengate_core`** (so even `python -c "import opengate"` blocks while +`G4*.tar.gz` archives are fetched into `core/opengate_core/geant4_data/`); only then is +there output. Do not mistake this silence for a hang. + +```bash +source "$OPEN_GATE_ENV/bin/activate" +cd "$OPEN_GATE_REPO" +opengate_tests -t actors/test008_dose_actor.py # path relative to opengate/tests/src +``` + +Test paths passed to `-t` are relative to `opengate/tests/src` **including the +subdirectory**, e.g. `actors/test008_dose_actor.py`, `source/test010_generic_source.py`. +A path that does not exist under `src/` is a fatal error +(`Explicit test paths must point inside the OpenGATE tests/src folder`), so check with +`find "$OPEN_GATE_REPO/opengate/tests/src" -name 'testNNN*.py'` first. Verified: this +command passes (`1/1`) on a correctly built environment. + +`opengate_info` prints resolver information (Geant4 version, data paths). Note it +**imports `opengate`**, so it fails with the same stale-`opengate_core` traceback until +the environment is consistent — it is not an independent health check. + +### 3.3 Check the **Geant4** version too (an environment property, not a repo bug) + +Appendix: A wrong Geant4 version is an **environment problem** — the repository is fine; +fix the environment. Before trusting any result, confirm the linked Geant4 matches the +project's pin (`GEANT4_VERSION` in `.github/workflows/main.yml`, currently `v11.4.2`): + +```bash +opengate_tests -l 2>&1 | head -5 # prints Detected / Required Geant4 version +``` + +Read it carefully — the runner only **warns**: + +``` +Detected Geant4 version: geant4-11-04 [MT] +Required Geant4 version: v11.4.2 +11 4 2 +11 4 0 +Geant4 version is not ok. This means the environment is not completely up to date +``` + +Note Geant4's own encoding: **`geant4-11-04` means 11.4.0**; patch 2 is spelled +`geant4-11-04-patch-02` (`G4VERSION_NUMBER 1142`). So a bare `geant4-11-04` is *not* the +required 11.4.2 — it is the older 11.4.0. If you see `Geant4 version is not ok`, the +environment is **not** equivalent to CI: tests may fail for environment reasons, and any +physics-sensitive comparison is unreliable. **Fix the environment** (do not change the repo): +update the Geant4 checkout to the pinned tag, rebuild it, then relink `opengate_core` +(§3.3, [`geant4-itk.md`](geant4-itk.md) §2–§4, §7) before drawing conclusions from a failing suite. + +Diagnose which Geant4 you actually have: + +```bash +cd # the checkout CMAKE_HOME_DIRECTORY points at +git describe --tags # e.g. 'v11.4.0' when the pin is v11.4.2 +grep '#define G4VERSION_NUMBER' source/global/management/include/G4Version.hh +# 1140 -> 11.4.0 ; 1142 -> 11.4.2 +``` + +This was a live finding on the current development environment (B-004 in +`skills/status/found-bugs.md`, classified there as an environment issue): the linked +Geant4 was 11.4.0 (`G4VERSION_NUMBER 1140`, `Geant4ConfigVersion.cmake` → +`set(PACKAGE_VERSION "11.4.0")`, checkout at tag `v11.4.0`) while CI pins 11.4.2. +The fix is `git fetch origin tag v11.4.2 && git checkout v11.4.2`, rebuild Geant4, then +relink `opengate_core`. + +The check itself is also defective (B-005, a genuine repo bug): `get_required_g4_version()` +reads a CI job name that does not exist and silently falls back to a hard-coded `v11.4.2`, +so the "Required" line is not really coming from `main.yml`. Compare against `main.yml` +yourself. + +### 3.4 Package version consistency check + +`uv pip list` / `pip list` must show both packages at the same version as `VERSION`, +and both editable installs must point at your working tree: + +```bash +VIRTUAL_ENV="$OPEN_GATE_ENV" uv pip list | grep -i opengate +# opengate $OPEN_GATE_REPO +# opengate-core $OPEN_GATE_REPO/core +``` + +(`pip list` with the environment activated is equivalent.) A mismatch — e.g. +`opengate` at one version next to a tree at another — means one editable install is stale. +Editable installs are `.pth` files pointing at the source, so the *Python* code is always +current — but a **previously compiled `.so` is not**: the C++ extension keeps its old +bindings until you rebuild, and `opengate` then fails at import on a missing +`opengate_core.Gate*Actor`. See [`geant4-itk.md`](geant4-itk.md) §7. + +## 4. Full path — build Geant4, ITK and `opengate_core` + +**This section moved to its own file: [`geant4-itk.md`](geant4-itk.md).** + +Read it only when you actually have to compile the native dependencies or rebuild the C++ +extension. The fast path above (§3) does not need any of it. Where each piece now lives: + +| You need to… | Where | +| --- | --- | +| Clone/build **Geant4** at the pinned version | [`geant4-itk.md`](geant4-itk.md) §2 | +| Build **ITK** | [`geant4-itk.md`](geant4-itk.md) §3 | +| Build **`opengate_core`** (bindings; `core/config.json`, parallelism, ccache/ninja) | [`geant4-itk.md`](geant4-itk.md) §4 | +| Install **`opengate`** against the source `opengate_core` | [`geant4-itk.md`](geant4-itk.md) §5 | +| Fix `cannot allocate memory in static TLS block` | [`geant4-itk.md`](geant4-itk.md) §6 | +| Fix a **stale `.so`** (`AttributeError: … has no attribute 'Gate…'`) | [`geant4-itk.md`](geant4-itk.md) §7 | + +The two facts you need most often, stated here so you do not have to open it: + +- **`$OPEN_GATE_DEPS` comes from the user** (§0) and must be on **non-volatile** storage — + Geant4 and ITK are multi-GB builds that must survive reboots. +- **An editable install never rebuilds the `.so`.** Any C++ change needs a rebuild before you + draw a conclusion; the symptom is a misleading `AttributeError` for a binding that *does* + exist in the sources ([`geant4-itk.md`](geant4-itk.md) §7). + +## 5. Optional extras — install ALL of them, then verify + +Several tests fail at import time without these; that is expected and not a regression. +Installing them is optional and requires the user's agreement (torch is a large download). + +> **Read this before installing.** The failure mode of this section is **installing only the +> package you happened to think of**. `torch` is the one people remember because it is the +> biggest, but the suite also needs `gaga_phsp`, `garf`, `pytomography`, `hist`, `pyvista` and +> `pymedphys`. Installing a subset leaves you with tests that are still skipped or still fail, +> and it is easy to misread that as a code problem. **Install the whole set, then run the audit +> in §5.1 to prove the environment is complete.** + +### 5.0 The authoritative list — take it from CI, not from memory + +`.github/workflows/actions_tests/action.yml` is what actually runs the suite in CI. Read it and +install exactly what it installs: + +```bash +cd "$OPEN_GATE_REPO" && grep -nE "pip install" .github/workflows/actions_tests/action.yml +``` + +At the time of writing that is (plus `SimpleITK`, and `torch` — see §5.2 for how to choose its +version, which is platform-dependent): + +```bash +source "$OPEN_GATE_ENV/bin/activate" # uv env: use `uv pip install` with VIRTUAL_ENV +uv pip install --upgrade torch +uv pip install "gaga_phsp>=0.7.6" # 'gaga-phsp' also resolves +uv pip install garf # needed by the garf/actor tests +uv pip install pytomography hist # needed by external/pytomography/* +uv pip install pyvista # needed by the pyvista-based tests +uv pip install pymedphys # needed by the contrib 'test075...' tests +uv pip install SimpleITK +``` + +Any install may legitimately pull its own transitive dependencies (e.g. `pytomography` brings +`kornia`, `fft-conv-pytorch`, `torchrbf`) — that is expected, not a surprise. + +### 5.1 Verify the environment is actually complete (do this every time) + +Do **not** decide by eye which packages are needed. Ask the test tree what it imports and check +each one is importable — this catches the package you did not think of: + +```bash +cd "$OPEN_GATE_REPO" && source "$OPEN_GATE_ENV/bin/activate" +python - <<'PY' +import ast, sys, pathlib, collections, importlib.util +src = pathlib.Path("opengate/tests/src") +local = {"opengate", "opengate_core", "utility", "gate", "tests"} | {p.stem for p in src.rglob("*.py")} +third = collections.defaultdict(set) +for f in src.rglob("*.py"): + try: + tree = ast.parse(f.read_text(encoding="utf-8", errors="ignore")) + except SyntaxError: + continue + for n in ast.walk(tree): + if isinstance(n, ast.Import): + for a in n.names: + third[a.name.split(".")[0]].add(f.name) + elif isinstance(n, ast.ImportFrom) and n.module and n.level == 0: + third[n.module.split(".")[0]].add(f.name) +missing = [(m, sorted(fs)) for m, fs in sorted(third.items()) + if m not in local and m not in sys.stdlib_module_names + and importlib.util.find_spec(m) is None] +print("NOTHING MISSING" if not missing else "") +for m, fs in missing: + print(f"MISSING {m}: {fs}") +PY +``` + +Expected output on a complete environment: **`NOTHING MISSING`**. Known false positives, safe to +ignore — these are **local modules of the test tree**, not PyPI packages (confirm with +`grep -rn "module \|import " opengate/tests/src | head`): + +| Reported as missing | What it really is | +| --- | --- | +| `actors` | a **directory** under `opengate/tests/src/` imported by one visu test | +| `test043_garf_helpers` | a sibling helper module imported by the `test043_garf_*_wip.py` files | +| `coresi` | `opengate/contrib/compton_camera/coresi_helpers.py`, a local module | + +Everything else in that list is a real gap — install it (§5.0). Do not proceed while a *real* +package is still missing: its tests will be skipped or fail, and you will be tempted to blame the +code. + +Then confirm the *count*: the runner reports how many files it will execute, and installing the +extras should raise it. On the machine where this was written the count went **368 → 376** once +the extras were in place: + +```bash +opengate_tests -t misc/test001_g4threevector.py | head -5 # look at the discovered-file count +``` + +A test that is still skipped prints its reason in the log (`--> Torch not avail`, +`--> pytomography not avail`): read it rather than assuming. + +### 5.2 `torch` and `numpy` have a **per-platform** compatibility constraint — search, never copy a version list + +**There is no portable “known good” set of `torch`/`numpy` versions for this project.** The +correct combination depends on the machine, so a version list copied from another session is a +bug waiting to happen. Determine it on the machine you are on. + +**Why it is platform-specific.** `torch` is compiled against a specific NumPy C-ABI. A `torch` +wheel built against NumPy 1.x does **not** interoperate with NumPy ≥ 2: importing still succeeds +and plain tensors still work, but *any* bridge call fails, which is what breaks the GAN/`garf` +tests: + +``` +A module that was compiled using NumPy 1.x cannot be run in NumPy 2.x … +RuntimeError: Numpy is not available # on torch.from_numpy / .numpy() / DataLoader +``` + +Whether you can escape that by upgrading `torch` depends entirely on the platform, because +PyTorch publishes wheels per OS/architecture — and **drops them for architectures it no longer +builds**, e.g. there is **no Intel-macOS (`x86_64`) wheel newer than `torch 2.2.2`**, whereas Linux +and Apple Silicon have current releases on NumPy 2. + +**The procedure — do this instead of copying versions:** + +```bash +source "$OPEN_GATE_ENV/bin/activate" + +# 1. What platform am I on, and is torch's numpy bridge actually broken? +python -c "import platform,sys;print(platform.platform(), platform.machine(), sys.version.split()[0])" +python -c "import torch,numpy as np;print(torch.__version__, np.__version__);\ +print(torch.from_numpy(np.arange(3.0)).numpy())" # fails => ABI mismatch + +# 2. What is the NEWEST torch this platform can have? (dry-run changes nothing) +uv pip install --dry-run --upgrade torch + +# 3. If a newer torch exists -> upgrade torch, keep numpy 2: +uv pip install --upgrade torch + +# 4. If torch is already at its newest -> you must go DOWN to numpy 1.x. +# Ask the resolver which numpy/partners are mutually consistent (no guesswork): +uv pip install --dry-run "numpy<2" torch +uv pip install --dry-run "numpy<2" torch pandas scipy # partner downgrades it implies + +# 5. Apply what the resolver agreed on, then RE-VERIFY the bridge (step 1). +``` + +Rules that follow: + +- **Let the resolver decide the partners.** On Intel macOS the coherent set was + `numpy 1.26.x` + `torch 2.2.2` **+ `pandas 2.x` + `scipy 1.12.x`** — `pandas 3` and `scipy ≥1.13` + pull NumPy ≥ 2 back in, so asking only for `numpy<2 torch` silently fails or diverges. Always + dry-run the *whole* set (step 4) and read what `uv` proposes. +- **Never leave a half-applied downgrade.** `uv` may abort the resolve and leave the environment + untouched; check the exit status and the resulting `uv pip list`, then re-run step 1. +- **Re-verify the bridge after every change** — a successful install is not a working pair. +- **Record what you found, not what you hoped:** put the *platform* next to the versions, or the + next agent will copy them onto a machine where they are wrong (this is the trap that produced + B-012). +- If the downgrade is unacceptable for other work on that machine, use a **separate throwaway + environment** for the torch tests rather than degrading the main one. + +**Known-observed example (so you can recognise the pattern, *not* to copy the versions):** on +`macOS-15.8-x86_64` (Intel) with Python 3.12, the newest available `torch` was `2.2.2`, and the +coherent set was `numpy 1.26.4` + `pandas 2.3.3` + `scipy 1.12.0` + `torch 2.2.2`. On Apple +Silicon and Linux the same project takes a current `torch` with NumPy 2 and needs **no** downgrade. +The platform half of that sentence is the part that matters. + +**Why CI does not catch this:** `.github/workflows/actions_tests/action.yml` installs torch on +`ubuntu-24.04` (`PLATFORM=x86_`), **`macos-15` (`PLATFORM=arm` — Apple Silicon)** and +`windows-2025`, i.e. only on platforms where PyTorch still ships current wheels. An Intel-Mac +developer therefore hits a failure CI structurally cannot reproduce — check the platform before +suspecting a `torch`-related test failure is a regression. + +Additionally CI installs `SimpleITK` explicitly and, on Linux/Windows, extends the +library path using the helper script: + +```bash +path=$(opengate_library_path.py -p site_packages) +export LD_LIBRARY_PATH="${path}/opengate_core.libs":${LD_LIBRARY_PATH} # Linux +# Windows (git-bash): export PATH="${path}\\opengate_core.libs":${PATH} +``` + +## 6. Build, packaging and CI +**This section moved to its own file: [`build-and-ci.md`](build-and-ci.md).** + +Read it when you **release a version**, touch wheels/packaging metadata, edit +`.github/workflows/`, or debug a **CI-only** failure. Where each piece now lives: + +| You need to… | Where | +| --- | --- | +| Understand the two packages and the `opengate-core==` pin | [`build-and-ci.md`](build-and-ci.md) §1 | +| Read the CI job topology and the workflow-level pins | [`build-and-ci.md`](build-and-ci.md) §2 | +| **Release** a version (bump `VERSION`, republish both, tag) | [`build-and-ci.md`](build-and-ci.md) §3 | +| Know what CI actually tests (the **wheels**, not your editable install) | [`build-and-ci.md`](build-and-ci.md) §4 | +| **Reproduce CI locally** | [`build-and-ci.md`](build-and-ci.md) §5 | +| Know the packaging consequences of touching `core/` | [`build-and-ci.md`](build-and-ci.md) §6 | +| Edit the workflows safely (single source for each pin) | [`build-and-ci.md`](build-and-ci.md) §7 | + +The fact that bites most often: **CI tests built wheels, not an editable install**, so a +wheel-only bug cannot reproduce locally unless you install the wheels too +([`build-and-ci.md`](build-and-ci.md) §5). + +## 7. Verification checklist (do this before reporting anything) + +```bash +cd "$OPEN_GATE_REPO" +cat user_secrets.json # must exist, real values, no CHANGE_ME +git check-ignore -v user_secrets.json # must be ignored +source "$OPEN_GATE_ENV/bin/activate" # MUST activate — see §3.1 +python -c "import opengate, opengate_core; print(opengate.__file__); print(opengate_core.__file__)" +python -m pip list | grep -i opengate # versions == VERSION (uv env: uv pip list) +opengate_tests -t actors/test008_dose_actor.py # one fast, representative test +``` + +- [ ] `user_secrets.json` exists, holds the **user's real** paths (no `CHANGE_ME`), and is + **not** tracked by git (`git check-ignore -v user_secrets.json`). +- [ ] `OPEN_GATE_REPO` / `OPEN_GATE_ENV` were loaded from it, and the environment is + **activated** (`which python` points into `$OPEN_GATE_ENV`). +- [ ] `$OPEN_GATE_ENV` is on **non-volatile** storage (`df -h "$OPEN_GATE_ENV"` shows a + real filesystem, not `tmpfs`). +- [ ] `opengate.__file__` points into `$OPEN_GATE_REPO`. +- [ ] `opengate_core.__file__` matches the install you intended (source build vs wheel). +- [ ] The **Geant4** version matches the CI pin (`opengate_tests -l | head -5`) — a + mismatch is only a warning, and it invalidates comparison with CI (§3.3, B-004). +- [ ] Both packages report the same version as `VERSION` (§3.4); if not, refresh the + install — a version-skewed editable `opengate_core` is the stale-`.so` case + ([`geant4-itk.md`](geant4-itk.md) §7). +- [ ] `ls "$OPEN_GATE_REPO/opengate/tests/data"` is non-empty, **and** the submodule HEAD + matches `git ls-tree HEAD opengate/tests/data` (§2.1) — otherwise reference-data tests + fail confusingly. +- [ ] One targeted test passes (expect `1/1 … 'True'`). +- [ ] `pip list` / `uv pip list` recorded in your report if you are filing a bug. + +A real, fully passing run looks like this (30-test slice, ~2.2 min): + +``` +Summary pass: 30/30 passed the tests: +Evaluation took in total: 2.2 min +True +``` + +## 8. Troubleshooting + +| Symptom | Cause / fix | +| --- | --- | +| `No module named pip` / no `bin/pip` | `uv`-created venv: use `uv pip` (§3.1). Also the cause of a sweepingly red build log that contains no compiler error — always check the exit code. | +| Every test fails with `ModuleNotFoundError: No module named 'opengate'`, including the auto-probe `misc/test001_g4threevector.py` | Venv not activated. The runner shells out to the literal command `python ` (§3.1). | +| `AttributeError: module 'opengate_core' has no attribute 'Gate...Actor'` | Stale compiled extension — rebuild `opengate_core` ([`geant4-itk.md`](geant4-itk.md) §7). | +| `RuntimeWarning: … registry differs from the linked Geant4` | Same stale binary as above ([`geant4-itk.md`](geant4-itk.md) §7) unless you genuinely edited the physics registry. | +| `Exception: Explicit test paths must point inside the OpenGATE tests/src folder` | `-t` path is wrong; it is relative to `opengate/tests/src` **with** the subdir (`actors/…`) (§3.2). | +| Silence for minutes, then Geant4 downloads | Expected on first `import opengate_core` (§3.2). | +| `ModuleNotFoundError: opengate_core` | You are in the wrong venv, or the C++ build failed. Re-run the install in `core/` ([`geant4-itk.md`](geant4-itk.md) §4) and read the CMake output. | +| CMake cannot find Geant4/ITK | `CMAKE_PREFIX_PATH` unset, not built yet, or wrong separator (`:` on Linux/macOS, `;` on Windows). | +| `cannot allocate memory in static TLS block` | See [`geant4-itk.md`](geant4-itk.md) §6, or rebuild Geant4 with `GEANT4_BUILD_TLS_MODEL=global-dynamic`. | +| Tests fail en masse with missing files | `opengate/tests/data` submodule not initialized (§2). | +| Download of Geant4/test data fails | `export GIT_SSL_NO_VERIFY=1` and retry. | +| Random test failures / crashes under load | CI runs many processes; reduce with `opengate_tests -n 4`. | +| Stale `*.so` after pulling C++ changes | Rebuild ([`geant4-itk.md`](geant4-itk.md) §7); delete `core/build/` if inconsistent. | +| Builds or downloads vanish after a reboot | The environment or deps live on volatile storage — move them to a non-volatile path the user provides (§0). | + +## 9. Clean-up / from-scratch reset + +Only ever remove paths the user agreed to remove; `$OPEN_GATE_ENV` and `$OPEN_GATE_DEPS` +may be shared with other projects. + +```bash +deactivate +df -h "$OPEN_GATE_ENV" # confirm which filesystem you are about to delete from +# rm -rf "$OPEN_GATE_ENV" # ask the user first +rm -rf "$OPEN_GATE_REPO/core/build" "$OPEN_GATE_REPO/core/CMakeFiles" "$OPEN_GATE_REPO/core/CMakeCache.txt" +cd "$OPEN_GATE_REPO" && git submodule update --init --recursive +rm -rf "$OPEN_GATE_REPO"/opengate/tests/output* "$OPEN_GATE_REPO"/opengate/tests/log +``` + +Never commit any of the removed paths — see `.gitignore`. \ No newline at end of file diff --git a/skills/environment-setup/build-and-ci.md b/skills/environment-setup/build-and-ci.md new file mode 100644 index 000000000..1b9e5c1c8 --- /dev/null +++ b/skills/environment-setup/build-and-ci.md @@ -0,0 +1,187 @@ +# Skill: `environment-setup/build-and-ci` + +**Goal:** know how OpenGATE is **built, packaged, released and tested in CI** — and reproduce that +process locally when a failure is CI-only. + +This is the deep-dive companion to [`SKILL.md`](SKILL.md). It is the single owner of the +*build/release/CI* knowledge. If you only need a working environment, you do **not** need this +file — `SKILL.md` §0–§5 are enough. + +Ownership boundaries: + +| Topic | Owner | +| --- | --- | +| How to *compile* Geant4, ITK and `opengate_core` | [`geant4-itk.md`](geant4-itk.md) | +| How to *package*, *release* and *test in CI* — and the packaging consequences of touching `core/` | **this file** | +| The runner, its options, its exit code | [`../running-tests/SKILL.md`](../running-tests/SKILL.md) | + +--- + +## 1. Two packages that must move together + +The repository ships **two** packages: + +| Package | Source | Contents | +| --- | --- | --- | +| `opengate` | `opengate/`, `pyproject.toml`, `setup.py` | pure Python | +| `opengate_core` | `core/`, `core/setup.py` | C++ (pybind11) binding Geant4 + ITK, **shipped as a compiled wheel** | + +`opengate` has a **hard pin** on its partner: `setup.py:15` sets +`install_requires=["opengate-core==" + version]`, where `version` is read from `VERSION` +(`core/setup.py:30` reads the same file). Consequence: **a version bump means republishing +both packages**, and any mismatch installs the wrong C++ layer. + +## 2. CI topology — read `.github/workflows/main.yml` + +The orchestrator is `main.yml`; the reusable pieces are composite actions. **Check the job +names in the file rather than trusting a summary** — they have changed before (B-005 was +caused by code reading a job name that no longer exists). + +Workflow-level pins (top of `main.yml`, in the `env:` block — this is the source of truth): + +```yaml +env: + GEANT4_VERSION: 'v11.4.2' + ITK_VERSION: 'v5.4.4' +``` + +| Job | Purpose | +| --- | --- | +| `build_opengate_wheel` | builds the pure-Python `opengate` wheel (one Python version is enough). | +| `build_opengate_core_wheel_pr` | builds the `opengate_core` wheel for the PR matrix (docker-based on Linux). | +| `build_opengate_core_wheel_ci_or_tag` | the CI/tag build of `opengate_core`. | +| `build_opengate_core_novis_wheel` | the no-visualisation variant (`opengate-core-novis`). | +| `publish_wheel` | splits and publishes wheels to PyPI — **only on a tag push**. | +| `test_wheel_pr` | installs the **built wheels** and runs the suite (see §4). | +| `publish_test` | publishes the results dashboard (master only). | + +Triggers: push/PR to `master` (PRs are cancelled in progress if superseded), a +**scheduled** run (`cron '0 0 * * 0,3'`, i.e. Sun/Wed), and `workflow_dispatch`. +**`docs/**` is in `paths-ignore`** — a docs-only change does not run CI. + +Helper files you may need to touch, all under `.github/workflows/`: +`actions_build/` (composite build action + `ci_build_wheel_{ubuntu,macos,windows}.sh`), +`actions_tests/action.yml`, the `Dockerfile_opengate_core*` images, +`createWheelLinux*.sh`, `delocateWindows.py`, `redoQt5LibsMac.py`. + +## 3. Releasing a version — the three steps + +`VERSION` at the repo root is the **single source of truth**; both `setup.py` files read it. +To release: + +1. **Bump `VERSION`** (e.g. `10.1.1` → `10.1.2`). +2. **Republish both packages.** `opengate` pins `opengate-core==` + (`setup.py:15`), so publishing `opengate` alone yields an unsatisfiable requirement — + CI would install nothing or fall back to a stale wheel. +3. **Tag the release.** `publish_wheel` only pushes to PyPI on + `github.event.ref` starting with `refs/tags/`; a plain branch push builds but does not + publish. + +Verify before tagging: + +```bash +cd "$OPEN_GATE_REPO" +grep -rn "opengate-core==" setup.py # the pin must match VERSION +diff <(cat VERSION) <(cd core && python -c "print(open('../VERSION').read()[:-1])") && echo "VERSION consistent" +``` + +Do **not** rename or remove a public parameter as part of a release without a deprecation path +(see [`engineering/python-gate-developer.md`](../engineering/python-gate-developer.md) §3). + +## 4. What CI actually does when it tests + +This is the part most often got wrong locally, because CI does **not** test an editable +install — it tests the **wheels**: + +- `actions_tests/action.yml` deletes the workspace, downloads the artifacts, and installs + `dist/opengate_core-*.whl` then `dist/opengate-*.whl`. A bug that only exists in the wheel + (missing file in `MANIFEST`, a stray import) therefore appears **only in CI**. +- Matrix: `{ubuntu-24.04, macos-15, windows-2025} × Python 3.10–3.14`, excluding macOS and + Windows on 3.10. +- Optional extras are installed explicitly: `torch`, `SimpleITK`, `gaga_phsp>=0.7.6`, + `pytomography`, `hist`. A test that needs them passes in CI and is skipped locally + (`SKILL.md` §5). +- On Linux/Windows the library path is extended via + `opengate_library_path.py -p site_packages` → `opengate_core.libs`. +- **The pass criterion is the last line of the runner output being exactly `True`** — the job + fails otherwise. This is why every `opengate_tests` invocation must end with `True`; + see [`running-tests/SKILL.md`](../running-tests/SKILL.md). +- Test selection: scheduled runs execute **everything**; push/PR runs use + `opengate_tests -r -s $sha` — `-r` runs **the last 10 tests plus 1/4 of the others, chosen + randomly** (see `opengate/bin/opengate_tests.py`), so a PR is *not* a full-suite guarantee. + `-s` seeds that random choice so a CI run is reproducible. +- The `opengate` wheel is built with `opengate/tests/data` **removed** and replaced by a + gitlink (`cp .git/modules/gam-tests/data/HEAD opengate/tests/`), so the binary test data is + not shipped in the wheel. + +## 5. Reproducing CI locally + +CI installs wheels into a scratch environment; do the same rather than testing your editable +install — a wheel-only bug cannot reproduce otherwise. + +```bash +cd "$OPEN_GATE_REPO" && source "$OPEN_GATE_ENV/bin/activate" # activation is required (SKILL.md §3.1) +python -m build # -> dist/ +pip install dist/opengate_core-*.whl dist/opengate-*.whl +export GIT_SSL_NO_VERIFY=1 +opengate_tests -r -s localsha # same shape as the PR job +``` + +`-s` is only a seed for the random test choice; CI passes a value derived from the commit SHA +and the OS/Python tag. + +Caveats: + +- This **replaces your editable installs with wheels** — do it in a scratch environment, not + the one you develop in, or you will silently lose the source build. +- The wheel pulls `opengate-core==` from PyPI if your local wheel is not visible; + install the locally built `opengate_core` wheel **first** (as above). +- Full-suite parity means installing the optional extras of `SKILL.md` §5. +- A CI-only failure is usually one of: a file missing from the wheel, an extra only CI has, + the `output_dashboard` path, or the platform's library path. + +## 6. Packaging consequences of touching `core/` + +The authoritative **build** procedure is [`geant4-itk.md`](geant4-itk.md) §4 (Geant4/ITK, +`core/config.json`, incremental rebuild). Only the packaging/CI consequences belong here: + +- Geant4/ITK are located via `core/config.json` (`G4INSTALL`, `ITKDIR`; git-ignored) or + through `CMAKE_PREFIX_PATH` when the file is absent. The environment also reads the + `G4INSTALL`/`ITKDIR` variables, but `config.json` **wins** when present. +- Parallelism is `os.cpu_count()`, overridable with `OPEN_GATE_BUILD_JOBS` + (B-007 fixed the old hard-coded `-j4`). +- **An editable install never rebuilds the `.so`** — the failure is a misleading + `AttributeError: module 'opengate_core' has no attribute 'Gate…'` + ([`geant4-itk.md`](geant4-itk.md) §7). Rebuild before concluding anything about a C++ change. +- A new binding must be registered in `core/opengate_core/opengate_core.cpp` **and** the + extension rebuilt, or it does not exist at runtime. + +## 7. Editing the workflows safely + +- **Change the pin in one place.** `GEANT4_VERSION` / `ITK_VERSION` live in the workflow-level + `env:` block of `main.yml`; the build scripts and the test action consume them. Do not + hard-code a second copy. +- **Keep job names and the code that reads them in sync.** B-005 is exactly this defect: + `get_required_g4_version()` in `opengate/bin/opengate_tests_helpers.py` indexes + `jobs.build_wheel`, a job that does not exist, and silently falls back to a literal. + If you rename a job, grep the Python for it. +- **CI validates against a hard-coded fallback today**, so a pin bump can silently stop being + enforced until B-005 is fixed — verify by reading `main.yml` yourself. +- **Secrets are needed for publish** (`PYPI_OPENGATE_CORE*`); a fork's PR cannot publish, and + publication only happens on tags (§3). +- Local runs cannot reproduce the docker-based Linux build + (`Dockerfile_opengate_core{,_arm64,_novis}`) — treat a Linux-wheel-specific failure as + needing a CI run, not a local guess. +- Keep `paths-ignore: docs/**` in mind: a docs-only PR does not exercise the build. + +## 8. Definition of done + +- [ ] The job names and pins you relied on were read from `main.yml`, not recalled. +- [ ] A version bump touched `VERSION` **and** both packages, and the `opengate-core==` pin matches. +- [ ] The release is tagged (`refs/tags/…`), not just pushed on a branch. +- [ ] A CI-only failure was reproduced with **wheels** in a scratch environment (§5), not with an + editable install. +- [ ] Any `core/` change was rebuilt before the packaging conclusion was drawn + ([`geant4-itk.md`](geant4-itk.md) §7). +- [ ] Workflow edits kept a single source for each pin and left job names greppable by the Python + that reads them. diff --git a/skills/environment-setup/geant4-itk.md b/skills/environment-setup/geant4-itk.md new file mode 100644 index 000000000..260826b5a --- /dev/null +++ b/skills/environment-setup/geant4-itk.md @@ -0,0 +1,424 @@ +# Skill: `environment-setup/geant4-itk` + +**Goal:** prepare the native dependencies `opengate_core` links against — **Geant4** and **ITK** — +and build the C++ extension itself. + +This is the deep-dive companion to [`SKILL.md`](SKILL.md). Read `SKILL.md` first for the parts that +are *not* here: obtaining the virtual environment (§0–§3), the fast Python-only path, the optional +extras (§5), troubleshooting (§8) and the clean-up procedure (§9). Come here only when you actually +have to compile Geant4/ITK or rebuild `opengate_core` — the fast path in `SKILL.md` §3 does not +need any of this. + +Related: [`build-and-ci.md`](build-and-ci.md) owns the *packaging and CI* consequences of the +build (§5 there); this file owns the *compilation*. + +--- + +## 1. What you need, and what you must ask the user + +You need: a C++17 compiler, CMake, ~30 GB of disk, and patience (parallel `make -j N`). +Optional: Qt6 (install it **before** Geant4 if you want `opengate_visu`). + +**macOS:** the dependency source depends on the **architecture**. + +| Architecture | Prefix | Qt6 / deps source | +| --- | --- | --- | +| **Intel** (`x86_64`) | `/usr/local` | **MacPorts** (`/opt/local`) — Homebrew no longer supports Intel macOS, so `brew` is a dead end here | +| **Apple Silicon** (`arm64`) | `/opt/homebrew` | Homebrew is fine and still supported | + +Only the Intel case is special. If you are on Intel and find `/usr/local/opt/qtbase` or any +other `/usr/local` reference in the libraries, read +[macOS Intel: use MacPorts, not Homebrew](#macos-intel-use-macports-not-homebrew-verified-failure-mode) +before doing anything else. On Apple Silicon the standard `brew` recipes apply and this +whole section can be skipped. + +Tell the two apart with `uname -m` (`x86_64` = Intel, `arm64` = Apple Silicon) — note that +`sysconfig.get_platform()` reports `macosx-*` and does **not** reveal the arch. + +Install `ccache` up front — see §4 for why it must be enabled *before* configuring: + +```bash +sudo port install ccache # macOS Intel +brew install ccache # macOS Apple Silicon, or Linuxbrew +sudo apt install ccache # Debian/Ubuntu +``` + +**Ask the user for `$OPEN_GATE_DEPS` first** (`SKILL.md` §0) — a non-volatile prefix, never `/tmp`. +Geant4 and ITK are each multi-GB builds that must survive reboots. Use subdirectories +named exactly `geant4.11-build` and `itk-build` inside it, because the commands below and +the resulting `CMAKE_PREFIX_PATH` assume that layout (CI caches the same shape under +`~/software`, see `.github/workflows/actions_build/action.yml`). + +Before building anything, check whether usable builds already exist there and **reuse +them** — it saves 30+ minutes: + +```bash +find "$OPEN_GATE_DEPS" -maxdepth 2 -name 'Geant4Config.cmake' 2>/dev/null +find "$OPEN_GATE_DEPS" -maxdepth 2 -name 'ITKConfig.cmake' 2>/dev/null +``` + +## 2. Geant4 `v11.4.2` + +```bash +mkdir -p "$OPEN_GATE_DEPS" +cd "$OPEN_GATE_DEPS" +git clone --branch v11.4.2 https://github.com/Geant4/geant4.git --depth 1 +mkdir geant4.11-build && cd geant4.11-build +cmake -DCMAKE_CXX_FLAGS=-std=c++17 \ + -DGEANT4_INSTALL_DATA=ON \ + -DGEANT4_INSTALL_DATADIR="$OPEN_GATE_DEPS/geant4/data" \ + -DGEANT4_USE_QT=ON \ + -DGEANT4_USE_OPENGL_X11=ON \ + -DGEANT4_USE_QT_QT6=ON \ + -DGEANT4_BUILD_MULTITHREADED=ON \ + -DGEANT4_BUILD_TLS_MODEL=global-dynamic \ + ../geant4 +make -j $(nproc) +``` + +Set `GEANT4_USE_QT` and `GEANT4_USE_OPENGL_X11` to `OFF` if Qt is not installed. +`GEANT4_BUILD_TLS_MODEL=global-dynamic` avoids the `cannot allocate memory in static +TLS block` error on some Linux distros at a ~10 % speed cost. + +### macOS Intel: use MacPorts, not Homebrew (verified failure mode) + +**This section applies to Intel macOS only** (`uname -m` = `x86_64`). On Apple Silicon +(`arm64`) Homebrew is supported and the usual `brew` install is fine. + +On Intel, Homebrew no longer supports the platform, so a `brew`-installed Qt can disappear +(or fail to install) and silently break a previously working environment. When that happens +the whole chain is broken at once — Geant4, ITK *and* `opengate_core` all record absolute +paths into `/usr/local/opt/...` at configure time, so deleting one Homebrew formula leaves +dangling references in every library. (On Apple Silicon the equivalent prefix would be +`/opt/homebrew`, which is a different failure mode and not covered here.) + +Symptoms (all point at the same cause): + +``` +ImportError: dlopen(.../opengate_core.cpython-312-darwin.so): Library not loaded: + /usr/local/opt/qtbase/lib/QtWidgets.framework/Versions/A/QtWidgets +ld: library 'fftw3_threads' not found # ITK with ITK_USE_SYSTEM_FFTW=ON +otool -L | grep /usr/local # any hit = stale Homebrew reference +``` + +Install the replacements once (needs `sudo`) and check the layout: + +```bash +sudo port install qt610-qtbase fftw-3 fftw-3-single xercesc3 ccache +ls -d /opt/local/libexec/qt6/lib/cmake/Qt6 # Qt6 CMake package +ls /opt/local/lib/libfftw3_threads.dylib /opt/local/lib/libxerces-c.dylib +``` + +MacPorts splits Qt under `/opt/local/libexec/qt6` (not `/opt/local/lib`), and its +`xercesc3` ships **no** CMake package config — both facts cause configure failures unless +handled explicitly: + +| Problem | Fix | +| --- | --- | +| `Failed to find XercesC (missing: XercesC_VERSION)` | pass `-DXercesC_INCLUDE_DIR=/opt/local/include -DXercesC_LIBRARY=/opt/local/lib/libxerces-c.dylib` | +| `Could not find ... Qt6OpenGLWidgets` | add **both** `/opt/local` and `/opt/local/libexec/qt6` to `CMAKE_PREFIX_PATH` (a bare `/opt/local` is not enough) | +| stale cache keeps the old Homebrew paths | configure into a **fresh** build dir (`mv build build.homebrew.bak && mkdir build`) | + +Reconfigure and rebuild Geant4 against MacPorts (a clean directory is required — an existing +cache keeps resolving `Qt6_DIR=/usr/local/lib/cmake/Qt6`): + +```bash +cd "$OPEN_GATE_DEPS/geant4" +mv build build.homebrew.bak && mkdir build && cd build +cmake -DCMAKE_CXX_FLAGS=-std=c++17 \ + -DCMAKE_PREFIX_PATH="/opt/local;/opt/local/libexec/qt6" \ + -DCMAKE_INSTALL_PREFIX="$OPEN_GATE_DEPS/geant4/install" \ + -DGEANT4_INSTALL_DATADIR="$OPEN_GATE_DEPS/geant4/data" \ + -DXercesC_INCLUDE_DIR=/opt/local/include \ + -DXercesC_LIBRARY=/opt/local/lib/libxerces-c.dylib \ + -DGEANT4_USE_QT=ON -DGEANT4_USE_QT_QT6=ON -DGEANT4_USE_OPENGL_X11=OFF \ + -DGEANT4_BUILD_MULTITHREADED=ON -DGEANT4_BUILD_TLS_MODEL=global-dynamic .. +make -j $(sysctl -n hw.ncpu) +``` + +The Geant4 source tree is the checkout root itself (`../`), not a `v11.4.2/` subdirectory. +Confirm the relink before moving on — **every** line must read `/opt/local`: + +```bash +for L in $(find "$OPEN_GATE_DEPS/geant4/build/BuildProducts" -name '*.dylib'); do + otool -L "$L" | grep -q '/usr/local' && echo "STILL BROKEN: $L" +done # expect: no output +``` + +#### ITK: point FFTW at MacPorts — but **not** with a global prefix path + +ITK is usually built with `ITK_USE_SYSTEM_FFTW=ON`, so it hard-codes +double-and-single-precision FFTW paths from the old prefix into `ITKConfig.cmake`. Passing +`CMAKE_PREFIX_PATH=/opt/local` to fix that **breaks the build** in a non-obvious way: + +``` +.../itkzlib-ng/adler32.c:11:23: error: expected ';' after top level declarator +``` + +ITK's `UseITK.cmake` does `include_directories(BEFORE ${ITK_INCLUDE_DIRS})`, and +`ITK_INCLUDE_DIRS` absorbs the FFTW include dir. A global `/opt/local/include` then lands +**ahead of ITK's bundled zlib-ng**, so `` resolves to MacPorts' instead and `Z_EXPORT` +/ `PREFIX` stay undefined. `ITK_USE_SYSTEM_ZLIB=OFF` is set — the system zlib must not win. + +Keep only `fftw3.h` in a private directory and pass **no** `CMAKE_PREFIX_PATH`: + +```bash +mkdir -p "$OPEN_GATE_DEPS/itk-fftw-include" +cp /opt/local/include/fftw3.h "$OPEN_GATE_DEPS/itk-fftw-include/" + +cd "$OPEN_GATE_DEPS/itk/build" +cmake -DCMAKE_PREFIX_PATH= \ + -DCMAKE_C_COMPILER_LAUNCHER=ccache -DCMAKE_CXX_COMPILER_LAUNCHER=ccache \ + -DFFTWD_BASE_LIB=/opt/local/lib/libfftw3.dylib \ + -DFFTWD_THREADS_LIB=/opt/local/lib/libfftw3_threads.dylib \ + -DFFTWF_BASE_LIB=/opt/local/lib/libfftw3f.dylib \ + -DFFTWF_THREADS_LIB=/opt/local/lib/libfftw3f_threads.dylib \ + -DFFTW_INCLUDE_PATH="$OPEN_GATE_DEPS/itk-fftw-include" . +make -j $(sysctl -n hw.ncpu) +``` + +Verify the shadowing is gone before compiling — the **first** `-I` must be the private dir: + +```bash +grep '^C_INCLUDES' "$OPEN_GATE_DEPS"/itk/build/Modules/ThirdParty/ZLIB/src/itkzlib-ng/CMakeFiles/zlib.dir/flags.make +# C_INCLUDES = -I$OPEN_GATE_DEPS/itk-fftw-include -I... itkzlib-ng ... <- correct +# C_INCLUDES = -I/opt/local/include ... <- wrong: zlib-ng will fail +``` + +ITK builds **static** libraries (`.a`), so unlike Geant4 it has no runtime dylib rebinding +to verify — only the link-time FFTW paths above. Finally rebuild `opengate_core` (§4) with +ccache enabled, then confirm **zero** stale-prefix references and that the extension imports: + +```bash +SO="$OPEN_GATE_REPO/core/opengate_core/opengate_core.cpython-$(python -c 'import sys;print(f"{sys.version_info.major}{sys.version_info.minor}")')-darwin.so" +otool -L "$SO" | grep -c '/usr/local' # expect: 0 +cd /tmp && python -c "import opengate_core as g4; print(g4.GateInfo.get_G4Version())" +``` + +Do **not** work around this by installing the `opengate-core` wheel: it is a frozen binary +and stops tracking local C++ changes, defeating the point of the editable install (§7). + +The version is pinned by the workflow (`env.GEANT4_VERSION` in +`.github/workflows/main.yml`); the runner warns when the linked Geant4 differs +(`SKILL.md` §3.3). Do not guess the pin — read it. + +## 3. ITK + +```bash +cd "$OPEN_GATE_DEPS" +git clone --branch v5.2.1 https://github.com/InsightSoftwareConsortium/ITK.git --depth 1 +mkdir itk-build && cd itk-build +cmake -DCMAKE_CXX_FLAGS=-std=c++17 -DBUILD_TESTING=OFF ../ITK +make -j $(nproc) +``` + +Note: the CI pins ITK `v5.4.4` (`env.ITK_VERSION` in `main.yml`); the developer guide +still shows `v5.2.1`. Either works locally — use the CI version if you need to match CI. + +## 4. `opengate_core` (C++ bindings) + +The build is driven by `core/setup.py` (setuptools + a custom `CMakeBuild`), **not** by +scikit-build-core. Two things are worth knowing before you run it: + +1. **How Geant4/ITK are located.** `setup.py` reads `core/config.json` (git-ignored, + `.gitignore:61`) and passes `-DGeant4_DIR=` / `-DITK_DIR=`. The file is + absent by default, in which case it passes those as **empty** values and CMake falls back + to discovering Geant4/ITK through `CMAKE_PREFIX_PATH` — which is why the command below sets + it. The explicit, recommended way is to create the file: + + ```json + { "G4INSTALL": "$OPEN_GATE_DEPS/geant4.11-build", "ITKDIR": "$OPEN_GATE_DEPS/itk-build" } + ``` + + (real absolute paths, no `$VARS`). The `G4INSTALL` / `ITKDIR` *environment* variables are + also read, but **only as the defaults** — a present `config.json` overrides them. +2. **Parallelism comes from `os.cpu_count()`.** `setup.py` passes `-j` to + `cmake --build`, overridable with `OPEN_GATE_BUILD_JOBS=` (e.g. to leave cores free). + Until B-007 was fixed this was hardcoded to `-j4`; if you are on an older commit, expect + only 4 jobs and add the override or build directly (below). + +```bash +cd "$OPEN_GATE_REPO/core" +export CMAKE_PREFIX_PATH="$OPEN_GATE_DEPS/geant4.11-build/:$OPEN_GATE_DEPS/itk-build/:${CMAKE_PREFIX_PATH}" +source "$OPEN_GATE_ENV/bin/activate" +python -m pip install -v -e . # uv env: VIRTUAL_ENV="$OPEN_GATE_ENV" uv pip install -v -e . +``` + +On Windows use `;` as the `CMAKE_PREFIX_PATH` separator, not `:`. + +The command creates (or reuses) `core/build/cmake.--/` and compiles +the module in place. It **reuses the same directory across runs**, so most of the build is +already incremental — but note `setup.py` always re-runs `cmake` first, which re-configures +against whatever `Geant4_DIR`/`ITK_DIR` resolve to. + +### Make repeat builds faster (ccache / ninja / ccmake) + +A cold compile is many minutes. Three supported speed-ups (all three tools were available on +the reference machine — verify on yours first): + +```bash +which ccache ccmake ninja +``` + +**Configuring/building the existing tree directly** is the most reliable speed-up: the cache +survives between runs, so only changed translation units rebuild, and you control parallelism. +This is also how to relink quickly after swapping Geant4/ITK (§6): + +```bash +cd "$OPEN_GATE_REPO/core/build/cmake.linux-x86_64-cpython-3.14" # adjust to your platform tag +cmake -DGeant4_DIR="$OPEN_GATE_DEPS/geant4.11-build" \ + -DITK_DIR="$OPEN_GATE_DEPS/itk-build" . # pick up the new deps +make -j $(nproc) # incremental, all cores +``` + +**ccache** avoids recompiling identical translation units even after a clean build, and +compliments the above. It must be enabled *before* configuring — a build with no compiler +launcher ignores it entirely: + +```bash +cd "$OPEN_GATE_REPO/core/build/cmake.linux-x86_64-cpython-3.14" +cmake -DCMAKE_C_COMPILER_LAUNCHER=ccache -DCMAKE_CXX_COMPILER_LAUNCHER=ccache . +make -j $(nproc) +ccache -s # hit rate should climb on subsequent builds +``` + +Verify whether ccache is actually wired in — an empty launcher means **no**, even when +`ccache` is installed: + +```bash +grep -E 'CMAKE_CXX_COMPILER_LAUNCHER|CMAKE_CXX_COMPILER:' "$OPEN_GATE_REPO"/core/build/cmake.*/CMakeCache.txt +# CMAKE_CXX_COMPILER:STRING=/usr/bin/c++ <- no launcher: ccache unused +``` + +To make the launcher stick for the pip-driven build too, configure it once in the build +directory (as above) and then re-run the pip install: the cached launcher is honoured when +CMake reuses that directory. + +**Ninja** (`-G Ninja`) builds faster than Make and computes staleness more precisely, but it +changes the recorded generator: only use it on a **fresh** build directory +(`rm -rf "$OPEN_GATE_REPO"/core/build`) and expect one full compile. + +**ccmake** is a terminal UI for inspecting/editing the cache — handy to confirm +`Geant4_DIR`/`ITK_DIR` after a change: + +```bash +ccmake "$OPEN_GATE_REPO/core/build/cmake.linux-x86_64-cpython-3.14" # c=configure g=generate t=advanced +``` + +## 5. `opengate` (Python) + +```bash +cd "$OPEN_GATE_REPO" +source "$OPEN_GATE_ENV/bin/activate" +python -m pip install -v -e . # uv env: VIRTUAL_ENV="$OPEN_GATE_ENV" uv pip install -e . +``` + +Because `opengate_core` is already installed from source, pip will not pull the wheel. +If pip *does* try to install `opengate-core==$(cat VERSION)` from PyPI, your editable +`opengate_core` is not visible in this environment — fix that instead of letting pip +overwrite it. + +## 6. TLS workaround (only if §2 was built without `global-dynamic`) + +```bash +export LD_PRELOAD="$OPEN_GATE_DEPS/geant4.11-build/lib/libG4processes.so:$OPEN_GATE_DEPS/geant4.11-build/lib/libG4geometry.so:${LD_PRELOAD}" +``` + +(Adjust the `lib/` sub-path to wherever Geant4 put its libraries; find them with +`find "$OPEN_GATE_DEPS/geant4.11-build" -name 'libG4processes.so'`.) + +## 7. Rebuild when the C++ extension is stale (verified failure mode) + +An editable `opengate_core` install picks up *Python* source changes immediately, but the +compiled extension `core/opengate_core.cpython--.so` is only +rebuilt when you re-run the install. Pulling C++ changes without rebuilding therefore +produces an import-time failure on a binding that *does* exist in the sources. + +Observed in a local dev environment (branch `agentic-skills`), before rebuilding: + +``` +AttributeError: module 'opengate_core' has no attribute 'GateDigitizerDeadTimeActor'. +Did you mean: 'GateDigitizerReadoutActor'? +``` + +raised from `opengate/actors/digitizers.py` (`class DigitizerDeadTimeActor(...)`), plus a +`RuntimeWarning: GATE PhysicsListBuilder registry differs from the linked Geant4` listing +physics lists as "missing C++ bindings". Both are symptoms of the same stale binary: +`GateDigitizerDeadTimeActor` exists in `core/opengate_core/opengate_lib/digitizer/` and is +registered in `core/opengate_core.cpp`. After rebuilding, both disappeared +and the suite went green. + +Diagnose by comparing the `.so` timestamp with the sources — an `.so` older than the +`.cpp` files it wraps is stale: + +```bash +ls -la "$OPEN_GATE_REPO"/core/opengate_core/*.so +cd "$OPEN_GATE_REPO" && git log -1 --format=%cd # compare with the source tree's last change +``` + +`opengate_tests` does **not** catch this: `check_environment()` +(`opengate/bin/opengate_tests_helpers.py`) only warns about the Geant4 version and checks +the data folder, never the freshness of `opengate_core`. + +Fix (from `core/`), then re-run the smoke test. Use the **incremental** path (§4): the +build tree already exists, so re-configuring and re-running `make` rebuilds only what changed +— far faster than a fresh `pip install`, and it uses all cores instead of the pinned `-j4`: + +```bash +cd "$OPEN_GATE_REPO/core/build/cmake.linux-x86_64-cpython-3.14" # adjust platform tag +cmake -DGeant4_DIR="$OPEN_GATE_DEPS/geant4.11-build" \ + -DITK_DIR="$OPEN_GATE_DEPS/itk-build" . +make -j $(nproc) +``` + +If the cache is missing or you also want the editable install refreshed, fall back to the +full command: + +```bash +cd "$OPEN_GATE_REPO/core" +export CMAKE_PREFIX_PATH="$OPEN_GATE_DEPS/geant4.11-build/:$OPEN_GATE_DEPS/itk-build/:${CMAKE_PREFIX_PATH}" +source "$OPEN_GATE_ENV/bin/activate" +python -m pip install -v -e . # uv env: VIRTUAL_ENV="$OPEN_GATE_ENV" uv pip install -v -e . +``` + +The compile is long (several minutes, single-threaded by default). Launch it detached +rather than in a foreground command that may time out, and follow the log — write the log +to a **non-volatile** file too, so it survives the session: + +```bash +cd "$OPEN_GATE_REPO/core" +setsid sh -c "source '$OPEN_GATE_ENV/bin/activate' && python -m pip install -v -e . > \"$OPEN_GATE_ENV/core_rebuild.log\" 2>&1" \ + < /dev/null > /dev/null 2>&1 & +tail -f "$OPEN_GATE_ENV/core_rebuild.log" # watch for '[100%]' and the final exit code +``` + +An existing `core/build/cmake.*/CMakeCache.txt` is reused, so `CMAKE_PREFIX_PATH` can be +omitted if the cache already points at Geant4/ITK builds (check with +`grep -E 'Geant4_DIR|ITK_DIR' "$OPEN_GATE_REPO"/core/build/*/CMakeCache.txt`). + +After the rebuild, also refresh `opengate` itself so the two versions match `VERSION` +(`SKILL.md` §3.4), then confirm: + +```bash +source "$OPEN_GATE_ENV/bin/activate" +cd "$OPEN_GATE_REPO" +opengate_tests -t actors/test008_dose_actor.py # expect: 1/1 passed, 'True' +``` + +## 8. Definition of done + +- [ ] `$OPEN_GATE_DEPS` came from the user (`user_secrets.json`), is on non-volatile storage, + and existing Geant4/ITK builds were checked for reuse first. +- [ ] macOS: on **Intel**, Qt6/FFTW/XercesC come from MacPorts and no compiled library + references the removed Homebrew prefix (`otool -L` shows no `/usr/local`). On **Apple + Silicon**, `/opt/homebrew` references are expected and fine. +- [ ] Geant4 is built at the version pinned in the workflow, with `global-dynamic` TLS (or the + §6 workaround is applied and recorded). +- [ ] ITK is built and its version is stated (CI pin vs. developer-guide version). +- [ ] `core/config.json` (or `CMAKE_PREFIX_PATH`) points at the right Geant4/ITK — the location + that actually wins is the one you verified. +- [ ] After any C++ change, the extension was **rebuilt** (§7) — no conclusion drawn from a stale + `.so`. +- [ ] `opengate_tests -t actors/test008_dose_actor.py` prints `Geant4 version is OK` and ends + `1/1` / `True`. +- [ ] `opengate` and `opengate_core` both match `VERSION` (`SKILL.md` §3.4). diff --git a/skills/running-tests/SKILL.md b/skills/running-tests/SKILL.md new file mode 100644 index 000000000..7c012b740 --- /dev/null +++ b/skills/running-tests/SKILL.md @@ -0,0 +1,157 @@ +# Skill: running-tests + +**Goal:** run the right tests, fast, and report results with real evidence. + +GATE 10 has its own test runner (`opengate_tests`). It is **not** pytest. Each test is a +standalone `testXXX_*.py` script that returns/prints a status; the runner executes them, +collects results and writes a JSON dashboard. + +**Two prerequisites, both verified as easy to get wrong** (details in +`skills/environment-setup`): + +1. **The environment must be the one the user designated** — `OPEN_GATE_ENV` from + `user_secrets.json` (root of the repo, git-ignored; see `skills/environment-setup` §0), + a non-volatile path — and it must be **activated**, not merely referenced by absolute + path. The runner shells out to the literal command `python `; an un-activated venv + makes every single test fail with `ModuleNotFoundError: No module named 'opengate'`. +2. **The environment must be consistent** — a stale compiled `opengate_core` fails at + import before the runner even starts (`skills/environment-setup` §4.6). + +--- + +## 1. Entry point and where tests live + +```bash +cd "$OPEN_GATE_REPO" # user's path, from user_secrets.json +export OPEN_GATE_ENV="$(python3 -c 'import json;print(json.load(open("user_secrets.json"))["OPEN_GATE_ENV"])')" +source "$OPEN_GATE_ENV/bin/activate" # MUST activate — see environment-setup §3.1 +opengate_tests [OPTIONS] # console script -> opengate/bin/opengate_tests.py +``` + +Tests: `$OPEN_GATE_REPO/opengate/tests/src/`, split into sub-directories (458 test files +in this tree: 368 to run, 90 ignored): + +``` +actors/ advanced_tests/ chemistry/ external/ geometry/ misc/ physics/ source/ +``` + +Helpers: `opengate/tests/utility.py` (`test_ok`, `assert_images_equal`, …). +Runner internals: `opengate/bin/opengate_tests_helpers.py`. + +## 2. Options you will actually use + +From `opengate/bin/opengate_tests.py`: + +| Option | Meaning | +| --- | --- | +| `--start_id/-i`, `--end_id/-e` | Numeric range of tests to run (`default="all"`). | +| `--test/-t PATH` | Run one explicit test, path relative to `opengate/tests/src`. **Repeatable.** | +| `--random_tests/-r` + `--seed/-s` | Run the last 10 tests + 1/4 of the others randomly. This is what CI uses. | +| `--processes_run/-p` | `'mp'` (default, multiprocessing), `'sp'`, `'legacy'`. | +| `--num_processes/-n` | Number of processes, default `all` cores. | +| `--run_previously_failed_jobs/-f` | Re-run only tests that failed in the previous dashboard. | +| `--no_log_on_fail` | Do not dump the log of failing tests to stdout. | +| `--print_last_test/-l` | Print the highest `testNNN` number and exit (useful for adding a new test). | +| `--g4_version/-v` | Developer override for the Geant4 version string check, e.g. `-v v11.4.2`. | + +Examples (verified against this tree — see the `-t` path warning below): + +```bash +opengate_tests -t actors/test008_dose_actor.py # one test +opengate_tests -t actors/test008_dose_actor.py -t misc/test011_mt.py +opengate_tests -i 1 -e 12 -n 4 # numeric slice, 4 processes +opengate_tests -f # only last-run failures +opengate_tests -p sp -n 1 # serial, deterministic, easy to debug +opengate_tests -l # highest test id (114 here) +``` + +**`-t` paths are relative to `opengate/tests/src` and must include the subdirectory** +(`actors/…`, `source/…`, `misc/…`). A path outside that folder aborts the whole run with +`Exception: Explicit test paths must point inside the OpenGATE tests/src folder`, and a +path that does not exist there aborts with `…were not found among available tests`. There +is no `source/test008_dose_actor.py` — `test008_dose_actor.py` lives in `actors/`. Find +the real path first: + +```bash +find "$OPEN_GATE_REPO/opengate/tests/src" -name 'test008*.py' +``` + +**Activate the environment before running** — the runner spawns each test as the literal +shell command `python ` (`opengate/bin/opengate_tests_helpers.py`), so an +un-activated venv makes every test fail with `ModuleNotFoundError: No module named +'opengate'`. See `skills/environment-setup` §3.1. + +**Never** run the whole suite as a first move: it is long (458 test files here; 368 to +run, 90 ignored), needs many cores, downloads Geant4 + test data on first run, and the +output is unreadable. Slice it. + +## 3. Running a single test standalone + +For interactive debugging, a test can be executed directly (from `$OPEN_GATE_REPO`, with +the environment activated): + +```bash +source "$OPEN_GATE_ENV/bin/activate" +cd "$OPEN_GATE_REPO" +python opengate/tests/src/actors/test008_dose_actor.py +``` + +The script prints `OK`/error lines and may write into `opengate/tests/output/`. This is +fine for debugging a single case, but **report suite results only from `opengate_tests`**, +which is what CI runs and what the dashboard tracks. + +When invoked through the runner, each test's output is also captured to +`opengate/tests/log/.log` (e.g. `opengate/tests/log/test008_dose_actor.log`), +which is the fastest place to read a failure. + +## 4. Interpreting results + +- The runner prints a summary; interleaved per-test logs appear on failure unless + `--no_log_on_fail` is set. +- Each test's full output is also written to `opengate/tests/log/.log` — the + fastest place to read a failure when the screen output is long. +- **Before believing a failure, rule out the environment**: a stale `opengate/tests/data` + submodule, a wrong Geant4 version, or a stale `opengate_core` all produce failures that + look like code bugs (see `skills/environment-setup` §2.1, §3.3, §4.6). A reference-data + failure reads like `Error while reading the file 'output_ref/…'` — check the submodule + commit before anything else. +- The JSON dashboard is written to `opengate/tests/output_dashboard/dashboard_output*.json`. + Keys are test paths, values are `[status, ...]` — an empty/`""` status means "not run + yet", which is exactly what `-f` reads to re-run failures. +- History of CI runs: +- A test returning non-`True` from its final check = failure. Read the log; do not guess. + +## 5. Writing a new test + +1. Find the current highest id: `opengate_tests -l`. +2. Create `opengate/tests/src//testNNN_short_name.py` (next free number, zero + padded, snake_case topic). +3. Import helpers: `from opengate.tests import utility`. +4. Structure: + - create the simulation with the public API (`gate.Simulation`, `gate.add_*`); + - run it; + - assert on real output (dose, phase space, produced particles, geometry params) + using `utility` helpers; + - end with a `utility.test_ok(condition)` style check so the runner sees a verdict. +5. Tests must be self-contained, use only `opengate/tests/data` (or generated data) as + input, and write to `opengate/tests/output/` only. +6. Keep runtimes small (seconds to a couple of minutes); CI runs the whole matrix. +7. If you add an example that is also user documentation, mirror it under `docs/` (see + `skills/documentation`). + +## 6. Keeping tests honest + +- Vary the seed / statistics when a test asserts on stochastic quantities; a test that + passes only with one seed is a bug in the test. +- Prefer asserting on *physics-relevant* quantities with justified tolerances over + exact float equality on simulation output. +- If the test needs torch/gaga-phsp/pytomography, note it in the docstring — CI installs + those, a bare dev env may not. + +## 7. Checklist before reporting a test result + +- [ ] Environment verified per `skills/environment-setup/SKILL.md`. +- [ ] Exact command line quoted in the report. +- [ ] Pass/fail taken from the dashboard or the summary, not from absence of output. +- [ ] For failures: the top of the traceback + the test's own assertion line included. +- [ ] Nothing generated was left in the git index (`git status` clean of outputs). \ No newline at end of file diff --git a/skills/status/found-bugs.md b/skills/status/found-bugs.md new file mode 100644 index 000000000..aaab2240d --- /dev/null +++ b/skills/status/found-bugs.md @@ -0,0 +1,662 @@ +# Found bugs (agent-maintained log) + +Every bug an agent notices but does **not** fix in the same change must be logged here +immediately. Do not silently work around a defect: log it, then decide with the user +whether to fix it now. + +This log is for *newly discovered* defects. Things already tracked upstream belong in +GitHub issues — link them, do not duplicate their content. + +## Two categories — do not confuse them + +| Category | Meaning | Where it is fixed | +| --- | --- | --- | +| **Repository bug** | A defect in the code, tests, docs or CI of **this** project. | A commit in this repository. | +| **Environment issue** | The local machine's setup is wrong or outdated (Geant4/ITK version, stale compiled build, missing extras, wrong venv). **Nothing in the repo is broken.** | Fix the environment — see `skills/environment-setup`. Do **not** "fix" the repo for it. | + +A failing or warning-laden run is **not** evidence of a repository bug until the environment +has been proven correct (see the checklist in `skills/environment-setup`). Most confusing +failures here are environment issues: a wrong Geant4 version, a stale compiled +`opengate_core`, an un-activated venv. + +--- + +## Entry template + +```markdown +### B-NNN — +- **Status**: open | confirmed | workaround | fixing | fixed (commit) | wontfix +- **Category**: repository bug | environment issue +- **Severity**: blocker | high | medium | low +- **Area**: e.g. `opengate/actors/doseactors.py`, `core/…`, `.github/workflows/…`, + or `/environment` for environment issues. +- **Symptom**: what the user/agent sees (exact error message). +- **Reproduce**: exact commands, minimal snippet, seeds. +- **Environment**: commit hash, OS, Python, Geant4/ITK versions, dev env or wheel. +- **Suspected cause**: hypothesis, with the file:line you inspected. +- **Workaround**: what to do meanwhile, if anything. +- **Related**: task ids in `task-list.md`, GitHub issue/PR links. +``` + +Include enough to reproduce without rediscovering the environment. Never paste long logs +inline — quote the first error line and the failing assertion. + +--- + +## Open + +### Repository bugs + +| id | severity | area | summary | status | related | +| --- | --- | --- | +| B-008 | low | `opengate/tests/utility.py` | `create_output_ref()` creates the reference directory with `exist_ok=True`, so when the test-data submodule is behind, the directory exists but is empty and the test fails with a misleading comparison error instead of “reference data missing”. | confirmed, open | T-021 | +| B-007 | medium | `core/setup.py` | The CMake build hardcoded a parallelism of **4** (`-j4` / `--parallel 4`), so a cold `opengate_core` build never used the machine's cores. | **fixed in this branch** | T-018 | +| B-002 | medium | `opengate/bin/opengate_tests_helpers.py` | The runner hardcodes `python ` instead of `sys.executable`, so an un-activated venv fails all tests with `ModuleNotFoundError: opengate`. | confirmed, open | T-007 | +| B-005 | low | `opengate/bin/opengate_tests_helpers.py` | `get_required_g4_version()` reads `jobs.build_wheel.env.GEANT4_VERSION`, a job that **does not exist**, so the required version silently falls back to a hard-coded `v11.4.2`. | confirmed, open | T-013 | +| B-006 | low | tests / `.gitignore` | A full-suite run leaves an untracked `simulation.json` in the repo root. | confirmed, open | T-015 | +| B-001 | medium | `opengate/bin/opengate_tests_helpers.py` | `check_environment()` does not detect a stale `opengate_core` nor a missing venv activation, so a broken environment looks like hundreds of failing tests. | open (UX gap) | T-008 | +| B-003 | low | `skills/` (our own docs) | The skills shipped a wrong `-t` example path, a `pip`-only install recipe for a `uv` venv, and omitted the activation requirement. | fixed in this branch | T-006 | +| U-003 | medium | `opengate/managers.py`, `core/…/GateImageBox.h`, `core/…/GateVoxelSource.cpp` | Three upstream code defects **confirmed in the tree**: #1096 loguru `logger.remove()`, #800 Geant4 private-header include, #1032 latent `VoxelSource` double free. | confirmed, open upstream, not fixed here | T-032 | +| U-006 | medium | `core/…/digitizer/GateDigiCollectionsRootManager.cpp`, `core/…/digitizer/GateDigiCollection.cpp` | Upstream #1143: **confirmed** serial ROOT write-out in MT — unlocked singleton, `SetNtupleMerging(true)` with the default 4000-entry basket, `AddNtupleRow` called per row. Performance-only, no wrong results. | confirmed, open upstream, not fixed here | T-032 | +| U-009 | low | `opengate/tests/src/contrib/test075_siemens_cios_alpha.py`, `source/test044_pbs_rot_transl.py` | Upstream test-quality debt: #844 (test asserts nothing), #933 (failure misread as flakiness; not in the Linux CI set). | open upstream | T-032 | + +### Environment issues (not repository bugs) + +| id | severity | area | summary | status | related | +| --- | --- | --- | --- | --- | --- | +| B-004 | high (for result validity) | local Geant4 build | The Geant4 checkout was at `v11.4.0` while the project pins `v11.4.2`, so the full-suite baseline was not CI-comparable. | **resolved** — Geant4 rebuilt at v11.4.2, `opengate_core` relinked; runner prints `Geant4 version is OK` | T-012, T-017 | +| B-009 | high (for result validity) | local `opengate/tests/data` submodule | The test-data submodule was checked out four commits behind the pointer recorded in the parent repo, so reference data for several tests was missing — producing the only 2 failures in the 366/368 baseline. | **resolved** — submodule updated to `9fabbdddf`; the same 2 tests now pass (`2/2`, `True`). Not a repository bug. | T-014, T-020 | +| B-012 | high (blocks 11 tests) | local `$OPEN_GATE_ENV` (`torch` vs `numpy` ABI) | `torch 2.2.2` was installed against `numpy 2.4.4`, so every NumPy↔torch bridge call raised `RuntimeError: Numpy is not available` and the 11 torch-dependent tests were skipped. | **resolved** — coherent set found by search. Not a repository bug, and **no portable version list exists**: the correct pair is per-platform. | T-034 | +| B-013 | blocker (whole env unusable) | macOS **Intel** local deps (Qt/FFTW/XercesC) | Homebrew no longer supports Intel macOS, so the `brew`-provided Qt (`/usr/local/opt/qtbase`), fftw and xerces-c vanished — `import opengate_core` died at `dlopen` with `Library not loaded: …/QtWidgets`, and ITK/`opengate_core` had recorded `/usr/local` paths at configure time. **Intel-only**; Apple Silicon (`/opt/homebrew`) is unaffected and still supported. | **resolved** — Geant4 + ITK reconfigured against **MacPorts** and `opengate_core` rebuilt editable; `otool -L` shows 0 `/usr/local` refs and the voxel/MT smoke tests pass. Procedure recorded in `skills/environment-setup/geant4-itk.md` (§2, macOS Intel). | T-012, T-017 | + +### Upstream GitHub issues (triaged — see §"Upstream issue triage" below) + +| Upstream | title (short) | triage verdict | category | logged as | +| --- | --- | --- | --- | --- | +| [#1104](https://github.com/OpenGATE/opengate/issues/1104) | `source.start_time=0` silently overridden | **no longer reproduces — already fixed** | repository bug (fixed) | U-001 | +| [#849](https://github.com/OpenGATE/opengate/issues/849) | Wrong interpolation in `GateGenericSource::UpdateActivityWithTAC` | **no longer reproduces — already fixed** | repository bug (fixed) | U-001 | +| [#1059](https://github.com/OpenGATE/opengate/issues/1059) | `> 2^31` primaries crash mid-simulation | **already fixed** — `GetPlatformMaxPrimariesPerRun()` + warning + test098 | repository bug (fixed) | U-010 | +| [#1107](https://github.com/OpenGATE/opengate/issues/1107) | Tubs vs Hexagon transmits ~25 % fewer photons | **NOT REPRODUCED** — ratio `1.0000` over 3 seeds | not a defect (as reported) | U-002 | +| [#857](https://github.com/OpenGATE/opengate/issues/857) | `RepeatParametrisedVolume` deletes ROOT output files | **NOT REPRODUCED** — repeater is innocent | not a defect (as reported) | U-004 | +| [#1035](https://github.com/OpenGATE/opengate/issues/1035) | `DigitizerHitsCollectionActor` ROOT file not finalized | **NOT REPRODUCED** — `End == size`, `uproot` reads `Hits;1` | not a defect (as reported) | U-004 | +| [#1115](https://github.com/OpenGATE/opengate/issues/1115) | `ShieldingLIQMD_HP_EMZ` not working in 10.1.1 | **NOT REPRODUCED** — configures and runs on 10.1.1/Geant4 11.4.2 | not a defect (as reported) | U-006 | +| [#1135](https://github.com/OpenGATE/opengate/issues/1135) | TLEDoseActor score spikes in 8-thread runs | **real race — fixed on master, in NO release tag** | repository bug (fixed on master) | U-006 | +| [#1143](https://github.com/OpenGATE/opengate/issues/1143) | Mutex contention on ROOT write-out | **CONFIRMED present** — cause identified in the code | repository bug | U-006 | +| [#800](https://github.com/OpenGATE/opengate/issues/800) | `GateImageBox.h` includes a Geant4 **private** header | **confirmed present in the tree** | repository bug | U-003 | +| [#1032](https://github.com/OpenGATE/opengate/issues/1032) | `VoxelSource` potential double free in MT | **confirmed present in the tree** | repository bug | U-003 | +| [#616](https://github.com/OpenGATE/opengate/issues/616) | `scale_itk_image` overrides the original ITK image | **confirmed fixed in the tree** (`array_from_image` = a copy) | repository bug (fixed) | U-005 | +| [#1145](https://github.com/OpenGATE/opengate/issues/1145) | PET crosstalk not implemented in Python | **confirmed: not in the tree** (feature gap) | feature request | U-007 | +| [#1096](https://github.com/OpenGATE/opengate/issues/1096) | `logger.remove()` kills the user's loguru handlers | **confirmed present in the tree** | repository bug | U-003 | +| [#942](https://github.com/OpenGATE/opengate/issues/942) | `Cannot import opengate_core` (missing Qt6 .so) | **environment / packaging issue** | environment issue | U-008 | +| [#938](https://github.com/OpenGATE/opengate/issues/938) | same Qt6 `cannot open shared object file`, `[novis]` does not help | **environment / packaging issue** | environment issue | U-008 | +| [#1000](https://github.com/OpenGATE/opengate/issues/1000) | ITK build fails on Linux with GCC 15 | **environment/doc gap** (`geant4-itk.md`) | environment issue | U-008 | +| [#844](https://github.com/OpenGATE/opengate/issues/844) | `test075_siemens_cios_alpha.py` is not a proper test | **confirmed** (test does not assert) | repository bug (tests) | U-009 | +| [#933](https://github.com/OpenGATE/opengate/issues/933) | flakiness on `source/test044_pbs_rot_transl.py` | **open upstream, test design** | repository bug (tests) | U-009 | +| [#1141](https://github.com/OpenGATE/opengate/issues/1141) | “Geant4 version is not ok” on `opengate_tests` | **already known — this is B-004/B-005** | environment + runner bug | B-004, B-005 | + +--- + +## Entries + + + +### B-001 — Stale compiled `opengate_core` breaks `import opengate` with a misleading `AttributeError` +- **Status**: fixed locally (rebuilt); the detection gap is still open (T-008) +- **Category**: environment issue (stale local build) + repository UX gap (detection) +- **Severity**: high (makes the entire suite unrunnable, with a message that misleads) +- **Area**: `core/opengate_core/*.so` (build artefact) vs. `opengate/actors/digitizers.py` +- **Symptom**: + ``` + File "opengate/actors/digitizers.py", line 574, in + class DigitizerDeadTimeActor(DigitizerWithRootOutput, g4.GateDigitizerDeadTimeActor): + AttributeError: module 'opengate_core' has no attribute 'GateDigitizerDeadTimeActor'. + Did you mean: 'GateDigitizerReadoutActor'? + ``` + preceded by `RuntimeWarning: GATE PhysicsListBuilder registry differs from the linked Geant4: * missing C++ bindings: FTFP_BERT, …`. +- **Reproduce**: `"$OPEN_GATE_ENV/bin/python" -c "import opengate"` (or + `opengate_tests -t actors/test008_dose_actor.py`) on an editable `opengate_core` whose + compiled extension predates the C++ sources. +- **Environment**: commit `8f049324`, Ubuntu, Python 3.14.7, editable installs of both + packages, `opengate_core` at one version against a tree at another; the `.so` was dated + four months **older** than the `.cpp` files it wraps. +- **Suspected cause**: `GateDigitizerDeadTimeActor` exists in `core/opengate_core/opengate_lib/digitizer/` and is registered in `core/opengate_core.cpp` (init + call), so this is **not** a missing binding in the sources — only the compiled `.so` is out of date. Editable installs pick up Python changes immediately but never re-run CMake on their own. +- **Workaround / fix**: `cd core && CMAKE_PREFIX_PATH=: … install -v -e .`, then refresh `opengate` too. Rebuild cleared both the `AttributeError` and the physics-list warning (they share the root cause). Verified: `import opengate` clean, `opengate_tests -t actors/test008_dose_actor.py` → `1/1` passed. +- **Why the runner missed it**: `check_environment()` only warns about the Geant4 version and checks the data folder; it never inspects `opengate_core` (T-008). +- **Related**: `skills/environment-setup` §4.6. + +### B-002 — Test runner hardcodes `python`, breaking un-activated venvs +- **Status**: confirmed, open (T-007) +- **Category**: repository bug (robustness), surfaced by an environment mistake +- **Severity**: medium (opaque mass failure; trivially avoided once known) +- **Area**: `opengate/bin/opengate_tests_helpers.py:344` (and `:381`) +- **Symptom**: every test fails, including the automatic first-run probe: + ``` + Running: misc/test001_g4threevector.py FAILED ! 0.0 s + … + ModuleNotFoundError: No module named 'opengate' + Summary pass: 0/1 passed the tests + ``` + while the runner process itself imported `opengate` successfully. +- **Reproduce**: run the environment's `opengate_tests` **by absolute path without + activating**, e.g. `"$OPEN_GATE_ENV/bin/opengate_tests" -t actors/test008_dose_actor.py`, + on a system where `python` resolves to the system interpreter rather than the venv's. +- **Environment**: commit `8f049324`, Ubuntu, a `uv` venv, system `python` at a different + installation than `$OPEN_GATE_ENV`. +- **Suspected cause**: `cmd = f"python {path_tests_src / f}"` is executed through the shell, so the test subprocess resolves `python` from `PATH` rather than using `sys.executable`. A pre-existing `FIXME` at line 340 documents this as intentional for Windows compatibility. +- **Workaround**: always `source "$OPEN_GATE_ENV/bin/activate"` before running the suite + (documented in `skills/environment-setup` §3.1). +- **Verified fix (not yet applied)**: replacing `cmd = f"python {path_tests_src / f}"` with an + argument list `[sys.executable, str(...)]` run via `subprocess.run(..., shell=False)` (as the + two in-code `FIXME`s ask, without POSIX quoting) makes B-002's exact repro pass: with + `which python` → **`python not found`** on this machine, + `"$OPEN_GATE_ENV/bin/opengate_tests" -t actors/test008_dose_actor.py` still reported `1/1` + and `True`. The patch was reverted because only `skills/`/`AGENTS.md` were in scope; the two + `FIXME` comments in `run_one_test_case` and `run_one_test_case_mp` still document it (T-029). +- **Related**: T-007, T-029, and `skills/environment-setup` §3.1. + +### B-003 — Skill documents disagreed with the code in three places +- **Status**: fixed in this branch +- **Category**: repository bug (our own docs) +- **Severity**: low (they cost real time to debug) +- **Area**: `skills/environment-setup/SKILL.md`, `skills/running-tests/SKILL.md`, `AGENTS.md` +- **Symptoms**: (1) the example `opengate_tests -t source/test008_dose_actor.py` aborts with `Exception: Explicit test paths must point inside the OpenGATE tests/src folder` — the file is `actors/test008_dose_actor.py`; (2) `python -m pip …` fails with `No module named pip` on a `uv`-created venv; (3) the activation requirement was not stated, which is what triggers B-002. +- **Suspected cause**: written from reading the tree, not from running it. `-t` paths are resolved relative to `opengate/tests/src` **including** the subdirectory (`select_tests_by_explicit_paths`). +- **Workaround / fix**: all three corrected this branch, each now citing the observed failure. +- **Related**: T-006. + +### B-012 — [ENVIRONMENT, not a repo bug] `torch` and `numpy` ABI mismatch blocks every torch test +- **Status**: **resolved** — coherent set found and installed; 9/9 torch tests pass +- **Category**: environment issue (package ABI mismatch), **not** a repository bug +- **Severity**: high *for test coverage* — 11 test files were silently skipped (`--> Torch not avail`) +- **Area**: `$OPEN_GATE_ENV` on `macOS-15.8-x86_64` (Intel Mac), Python 3.12 +- **Symptom**: plain tensors worked, but *any* NumPy↔torch bridge call failed, which is what every + GAN/`garf` test needs: + ``` + A module that was compiled using NumPy 1.x cannot be run in NumPy 2.x … + RuntimeError: Numpy is not available # on torch.from_numpy / .numpy() + ``` + Triggered by `torch.from_numpy(np.arange(3.0)).numpy()` with `torch 2.2.2` against + `numpy 2.4.4`. +- **Cause**: `torch` is compiled against a specific NumPy C-ABI; `torch 2.2.2` is a NumPy-1.x build + and cannot interoperate with NumPy ≥ 2. The obvious fix — upgrade `torch` — **is not available + on this platform**: PyTorch publishes no Intel-macOS (`x86_64`) wheel newer than `2.2.2` + (`uv pip install --dry-run --upgrade torch` offers nothing newer), whereas Linux and Apple + Silicon have current NumPy-2 builds. +- **Why CI never sees it**: `.github/workflows/actions_tests/action.yml` installs torch only on + `ubuntu-24.04` (`PLATFORM=x86_`), **`macos-15` (`PLATFORM=arm` — Apple Silicon)** and + `windows-2025` — all platforms where PyTorch still ships current wheels. This is a + developer-machine-only failure that CI structurally cannot reproduce. +- **Fix applied** (resolver-driven, **not** copied from anywhere — see + [`environment-setup/SKILL.md`](../environment-setup/SKILL.md) §5.2 for the procedure): + `numpy==1.26.4` + `pandas==2.3.3` + `scipy==1.12.0` + `torch==2.2.2`. Asking only for + `numpy<2 torch` is not enough: `pandas 3.x` / `scipy ≥1.13` pull NumPy ≥ 2 back in, so the whole + partner set must be dry-run together. +- **Verification**: bridge OK (`torch.from_numpy(...).numpy()`); `opengate`, `opengate_core`, + `gaga_phsp 0.7.6`, `garf 2.10` all import; **`opengate_tests` now discovers 376 files instead of + 368**. Torch suite: `test081_simulation_optigan_with_random_seed.py` → `1/1`, `True`; the 7-test + GAN/`garf` batch (`test034`, `test038_gan_phsp_spect_gan_{aa,se,ze,mt}`, `test040`, + `test047`) → `Summary pass: 8/8`, `True` including the MT variant. +- **Portability warning — this is the important part**: the four versions above are + **machine-specific** and must never be copied to another machine. Record the *platform* with + them, and re-derive the set with the §5.2 procedure. +- **The follow-on lesson (this is the one worth keeping)**: after fixing `torch` I reported the + `pytomography`/`hist` tests as “still uncovered” — but they were simply **not installed**, and CI + installs both (`actions_tests/action.yml`). The skill's §5 listed the extras without a complete + inventory and without any verification step, so a partial install looked like success. §5.0 now + takes the list from CI and §5.1 audits the whole test tree and must print `NOTHING MISSING`. + `pytomography`, `hist`, `pyvista` and `pymedphys` were then installed and + `external/pytomography/test094d_reconstruction.py` → `1/1`, `True` (128.8 s). +- **Related**: T-034, T-009, [`environment-setup/SKILL.md`](../environment-setup/SKILL.md) §5.0–§5.2. + +### B-004 — [ENVIRONMENT, not a repo bug] Geant4 11.4.0 installed where CI pins 11.4.2 +- **Status**: open — must be fixed by upgrading the **local Geant4 build** (T-012) +- **Classification**: **environment configuration issue, not a defect in this repository.** + The Geant4 checkout simply sits on an older release; no source change here is needed. +- **Severity**: high *for the validity of local results* — a full-suite run in this + environment is not comparable with CI. +- **Area**: the Geant4 source/build referenced by `$OPEN_GATE_DEPS`, linked into + `opengate_core`. +- **Symptom**: every `opengate_tests` run prints the warning + `Geant4 version is not ok. This means the environment is not completely up to date` + (`Detected: geant4-11-04 [MT]`, `Required: v11.4.2`). +- **Cause**: the Geant4 checkout is at tag **`v11.4.0`** (branch `geant4-11.4-release`, commit + `b4a16de652`, header `#define G4VERSION_NUMBER 1140`). Geant4 spells 11.4.0 as + `geant4-11-04` and patch 2 as `geant4-11-04-patch-02` (`G4VERSION_NUMBER 1142`), so the + detected string is correct and the build is genuinely one patch release behind. +- **Fix**: update the Geant4 source to `v11.4.2`, rebuild it, then relink `opengate_core` + (see the environment guide for the build and relink steps). +- **Note**: this was initially logged as a repository bug; it is not. Only B-005 below is an + actual code defect. +- **Related**: T-012. + +### B-009 — [ENVIRONMENT, not a repo bug] test-data submodule behind the recorded pointer +- **Status**: **resolved** — `git submodule update --init opengate/tests/data` (now at `9fabbdddf`) +- **Category**: environment issue (submodule checkout state), **not** a repository bug +- **Severity**: high *for result validity* — it produced the only 2 failures in the 366/368 baseline +- **Area**: `opengate/tests/data` (binary submodule) vs. the pointer recorded in the parent repo +- **Symptom**: `geometry/test102_gammex467.py` fails with a data-comparison error and + `geometry/test107_macaco1_mt.py` fails on missing reference data, while the code and the Geant4 + build are both correct. +- **Cause**: the submodule was checked out four commits behind the pointer recorded in the parent + repo, so `output_ref/` directories were absent (see B-008 for why that misleads). +- **Fix**: `git submodule update --init opengate/tests/data`; the two tests then pass + (`Summary pass: 2/2`, `True`, logs ending with “Great, tests are ok.”). +- **Related**: B-008 (the misleading reporting), T-014, T-020. + +### B-005 — Required Geant4 version is read from a non-existent CI job +- **Status**: confirmed, open (T-013) — **fix verified but not applied** (out of scope this stage) +- **Category**: repository bug (real code defect) +- **Severity**: low (the hard-coded fallback happens to be correct today) +- **Area**: `opengate/bin/opengate_tests_helpers.py`, `get_required_g4_version()` (line 74–81) +- **Symptom**: the function is supposed to read the pinned version from `main.yml`, but: + ```python + g4 = githubworfklow["jobs"]["build_wheel"]["env"]["GEANT4_VERSION"] + ``` + raises `KeyError: 'build_wheel'` — the job names are `build_opengate_wheel`, + `build_opengate_core_wheel_pr`, … — so the `except`/fallback path returns the hard-coded + `"v11.4.2"` instead of the real CI pin. +- **Verified**: `GEANT4_VERSION` sits in the **workflow-level** `env:` block of `main.yml` + (a sibling of `jobs:`), confirmed by reading the file. A further latent defect was found: + `tests_dir.parents[2]` raises `IndexError` on any path fewer than 3 levels deep, *before* + the `fpath.exists()` guard runs — so the ``"Safe fallback"`` is unreachable for shallow + paths. +- **Fix prepared and tested** (reverted, pending scope): read + `github_workflow["env"]["GEANT4_VERSION"]` with a `(KeyError, TypeError)` guard, and + length-check `tests_dir.parents` before indexing. Runtime proof on the prepared patch: + real path → `v11.4.2`; with the pin temporarily edited to `v11.9.9` the call returned + `v11.9.9`, showing it now tracks the file rather than the literal; shallow path → fallback + instead of `IndexError`. +- **Reproduce**: parse `.github/workflows/main.yml` and index `jobs.build_wheel.env`. +- **Impact**: if CI's `GEANT4_VERSION` is bumped, the test runner keeps validating against + the stale literal, so the check can no longer protect the suite. +- **Related**: B-004, T-013. + +### B-006 — Full-suite run leaves an untracked `simulation.json` in the repository root +- **Status**: confirmed, open (T-015) +- **Category**: repository bug (test hygiene / missing ignore rule) +- **Severity**: low (pollutes `git status`; risks being committed by accident) +- **Area**: the test that serialises the simulation dump + `.gitignore` +- **Symptom**: after a full `opengate_tests` run, `git status` reports an untracked + `simulation.json` at the repository root (a serialised simulation dump beginning + `{"user_info": {…}}`). `git check-ignore` does **not** match it, and the file is dated + during the run. +- **Reproduce**: run `opengate_tests` from `$OPEN_GATE_REPO`, then `git status --short`. +- **Impact**: an agent following "never commit generated artifacts" must notice it by hand; + otherwise it can slip into a commit. +- **Suggested fix**: make the offending test write under `opengate/tests/output*/` (already + ignored), or add the produced filename to `.gitignore`. +- **Related**: T-015. + +### B-008 — Missing reference data looks like a failing test, not a missing dataset +- **Status**: confirmed, open (T-021) +- **Category**: repository bug (error-reporting quality) +- **Severity**: low (misdiagnosis trap; the data itself is fine upstream) +- **Area**: `opengate/tests/utility.py`, `create_output_ref()` (and the tests using it) +- **Symptom**: with the test-data submodule behind, `geometry/test102_gammex467.py` fails + with a data-comparison error and `geometry/test107_macaco1_mt.py` fails on a missing + reference, even though nothing is wrong with the code or the Geant4 build. The log shows + the reference directory existing but empty: + ``` + Exception: Error while reading the file 'output_ref/test102_gammex467/gammex467.mhd' + ``` +- **Reproduce**: check out `opengate/tests/data` at an older commit + (`git -C opengate/tests/data checkout 1f89d1f`) and run the two tests above. +- **Root cause of the confusion**: `create_output_ref()` calls + `mkdir(parents=True, exist_ok=True)`, so a missing dataset yields a **present but empty** + directory rather than an obvious “reference data missing” state. Note `git` never tracks + empty directories, so `git status` inside the submodule looks clean too. +- **Confirmed not a repository bug**: after `git submodule update --init opengate/tests/data` + (now at `9fabbdddf`), both tests pass — `Summary pass: 2/2`, `True`, logs ending with + “Great, tests are ok.”, including the test107 physics assertion + (FWHM within tolerance). +- **Suggested improvement**: have the reference-data helper assert that the expected files + exist and raise a clear “reference data missing — update the submodule” error. +- **Related**: B-009 (the environment cause), T-014, T-021. + +### U-001 — Upstream start-time / TAC-interpolation bugs are already fixed in this tree +- **Status**: verified fixed — nothing to do, **do not re-log as open** +- **Category**: repository bug (already resolved upstream in this tree) +- **Upstream**: [#1104](https://github.com/OpenGATE/opengate/issues/1104) and + [#849](https://github.com/OpenGATE/opengate/issues/849) — both still shown **open** on GitHub, + so they look like live defects; they are not. +- **Area**: `opengate/sources/base.py` `SourceBase.initialize_start_end_time` (line 117), and + `core/opengate_core/opengate_lib/GateGenericSource.cpp` `UpdateActivityWithTAC` (line 108). +- **What the report says**: (a) #1104 — `if not self.start_time:` treats `start_time = 0.0` as + unset, so every job of a split simulation resets the decay clock to its own run start. + (b) #849 — the TAC bin index is one too high and the two interpolation weights are swapped. +- **Checked in the current tree** (`996572b64`): `base.py:117` reads `if self.start_time is None:` + — exactly the fix #1104 asks for (`git log -L 112,121:opengate/sources/base.py` → commit + `100401286` “Fix start_time/stop_time default handling in SourceBase”). `GateGenericSource.cpp` + now clamps with `if (i == 0) { fActivity = fTAC_Activities[0]; return; }`, then `i -= 1` + (the “move to the lower bin edge” correction) and weights + `fTAC_Activities[i] * (t[i+1]-time)/dt + fTAC_Activities[i+1] * (time-t[i])/dt` — the same + algebra as the “GOOD INTERPOLATION” in the report. +- **Action**: nothing in the repo. The upstream issues can be closed; the *only* reason to touch + them here is the `end_time` sibling, which was fixed in the same commit. +- **Related**: — + +### U-002 — `Tubs` vs area-matched `Hexagon` transmission gap: **NOT REPRODUCED** +- **Status**: **dismissed as reported** — my controlled reproduction gives ratio **1.0000** +- **Category**: not a defect (on the evidence available here) +- **Severity**: n/a (was logged as high if real; the measurement below says it is not real) +- **Upstream**: [#1107](https://github.com/OpenGATE/opengate/issues/1107) (opened 2026-07-28, + OpenGATE 10.1.0, Python 3.10, Ubuntu 22.04) +- **What the report claims**: two holes of *exactly* matched open cross-sectional area + (0.5261 mm²), one `Tubs` one `Hexagon`, in tungsten; a parallel 140 keV beam gives 10.69 % vs + 8.02 % transmission under normal incidence (ratio 0.750, expected ≈ 1.0), stable at ~0.73–0.75 + across 9 isolation variants. +- **What I ran** (commit `996572b64`, Geant4 11.4.2, Python 3.12, macOS x86_64, 1 thread): + a boolean `Box(G4_W) − hole` for each shape, each hole sized **from the same target area** + (`Tubs` `rmax = sqrt(A/pi) = 0.40922 mm`; `Hexagon` circumradius `0.45 mm`, i.e. across-flats + `0.77942 mm`), a **true parallel beam** along +Z (`direction.type = "momentum"`, + `momentum = [0,0,1]`), and a `PhaseSpaceActor` on a 10×10 mm plane immediately behind the block. +- **Result**: `tubs = 500032`, `hex = 500026` out of 500 000 primaries each → **ratio 1.0000**. + Re-run with seeds 1, 2 and 3: **ratio 1.0000 each time**. No gap of any size, let alone 25 %. +- **Harness validation** (mandatory — see U-004's note): the same beam through an *empty* air + world transmits ~100 % (50 003/50 000), so the 0.0 % failures I first saw were my own bug, not + the code's. The final numbers come from a harness that passes that check. +- **Likely explanation of the original report**: the reporter's `Hexagon` radius convention. + GATE's `Hexagon.radius` is the **circumradius**; using it as the across-flats changes the open + area by a factor of **3**. Alternatively their beam was not truly parallel (see + [`engineering/gate-physics-expert.md`](../engineering/gate-physics-expert.md) §4.1 — `beam2d` + with `sigma=0` is *not* a parallel beam). Either way the effect is in the *setup*, not in + `G4Tubs`/`G4Polyhedra` construction. +- **Caveat**: I did not run the reporter's attached script and did not reproduce their exact + 9-variant lattice. My conclusion covers the *geometric* claim (equal-area holes transmit + equally), which is what the issue is about. +- **Related**: [`engineering/gate-geometry-expert.md`](../engineering/gate-geometry-expert.md) §4.2 + (the area formulas and the circumradius trap). + +### U-003 — Three upstream code defects confirmed in the tree (not fixed here) +- **Status**: confirmed present at commit `996572b64`; **not fixed in this change** (out of scope) +- **Category**: repository bug +- **Severity**: [#1096](https://github.com/OpenGATE/opengate/issues/1096) medium (silently + discards the *user's* logging), [#800](https://github.com/OpenGATE/opengate/issues/800) medium + (blocks source builds without Geant4 private headers), [#1032](https://github.com/OpenGATE/opengate/issues/1032) + medium (latent double free, MT-only) +- **Confirmed here**: + - [#1096](https://github.com/OpenGATE/opengate/issues/1096) — `_setter_hook_verbose_level` calls + a bare `logger.remove()` (`opengate/managers.py:1772`) when `log_handler_id == -1`. Loguru's + `remove()` with no argument removes **every** handler, including handlers the user added + themselves, so `logger.add('run.log')` is silently dropped on the first simulation. The fix is + to remove only the id GATE itself added (it already does this in the `else` branch, line 1776). + - [#800](https://github.com/OpenGATE/opengate/issues/800) — `core/opengate_core/opengate_lib/GateImageBox.h:35` + has `#include `, a Geant4 **internal** header that is not + installed by a normal Geant4 installation. It is guarded by `USE_VISU`/`G4VERSION_NUMBER`, + so a no-visu build is unaffected, but any build enabling Qt/OpenGL against a stripped Geant4 + fails. Reported as breaking builds; no in-tree workaround. + - [#1032](https://github.com/OpenGATE/opengate/issues/1032) — `GateSingleParticleSource`'s + destructor `delete`s `fPositionGenerator`, and `GateVoxelSource.cpp` hands it a + `fVoxelPositionGenerator` via `SetPosGenerator`, which the caller still owns. Benign only + while that destructor never runs on a worker thread — i.e. a latent double free, not yet a + crash. Upstream labels it `bug`. +- **Action**: not fixed here. Each needs its own branch, a test that fails first, and (for #800) + a confirmed skip-if-no-private-headers path. +- **Related**: this is the same *class* as the local-logging work in + [`engineering/simulation-debugger.md`](../engineering/simulation-debugger.md). + ([#1059](https://github.com/OpenGATE/opengate/issues/1059) was in this group and is now + **fixed** — see U-010.) + +### U-004 — Two upstream actor-output claims: **NOT REPRODUCED** +- **Status**: **both dismissed as reported** at commit `996572b64` +- **Category**: not a defect (on the evidence available here) +- **Severity**: n/a (were logged as high if real) +- **Upstream**: [#857](https://github.com/OpenGATE/opengate/issues/857) (opened 2025-12-04) and + [#1035](https://github.com/OpenGATE/opengate/issues/1035) (opened 2026-05-23). +- **What they claim**: + - #857 — merely *constructing* a `RepeatParametrisedVolume` (never added to the volume manager) + causes every digitizer `.root` file to be deleted at the end of the run, while `stat.txt` + survives. + - #1035 — a `DigitizerHitsCollectionActor` `.root` file is left **unfinalized** (ROOT header + `End == 252`, `uproot.open(f).keys() == []`) with a complex phantom and/or several modules. +- **#857 — what I ran**: the reporter's own two-case script (with and without the + `RepeatParametrisedVolume(...)` construction line), each writing to its own temp dir. + **Result: the ROOT file was absent in BOTH cases** — so the repeater is *not* the cause. + The real reason was that the source (a 100 kBq point source in a 1 m air world) produced **zero + hits** in the 10 cm detector, and GATE then reports + `⚠️ Empty output, no particles stored in …root` and correctly writes no file. Once the source + actually irradiates the detector, the `.root` file is produced and readable + (`uproot keys: ['Hits;1']`). **The repeater does not delete anything.** +- **#1035 — what I ran**: a NEMA-like phantom (water cylinder + 22 mm sphere) plus a 35×5×315 mm + CZT module, a 140.5 keV source inside the phantom, **2 threads** and 20 000 primaries, exactly + the trigger conditions reported. **Result: the file is correctly finalized** — ROOT header + `End` equals the file size (not 252) and `uproot.open(...).keys()` returns `['Hits;1']`. + **Not reproduced.** +- **Honest caveat**: #1035's reporter sees the failure only in *their* larger geometry (more + modules / more primaries). My reduced case is a faithful trigger but not a bit-for-bit copy of + theirs, so I can say "not reproduced here" — not "impossible". +- **Harness note (this is why the section exists)**: my first #857 attempt produced exactly the + symptom the issue describes — *no ROOT file* — and it was **my own harness**, not the code. Only + after validating the harness (does the same beam deposit energy in the detector at all?) did the + real picture appear. The traps I hit are now catalogued in + [`engineering/simulation-debugger.md`](../engineering/simulation-debugger.md) §5.1. +- **Note on the related closed issues**: `#1034`/`#1033` (same title as #1035) and `#1109` (“Actor + on Repeated Volume…”) were all closed as completed, so this area has changed since the reports. +- **Related**: B-006 (another “test leaves/takes files” hygiene bug), T-015. + +### U-005 — `scale_itk_image()` aliasing: **already fixed in the tree** +- **Status**: **fixed** — the code now takes a copy, which is what the issue asked for +- **Category**: repository bug (already resolved) +- **Severity**: n/a +- **Upstream**: [#616](https://github.com/OpenGATE/opengate/issues/616) +- **Checked in the tree**: `opengate/image.py:361` — `scale_itk_image` now starts with + `imgarr = itk.array_from_image(img)`, i.e. a **copy**; the report's premise + (`array_view_from_image`, which aliases) no longer holds. The neighbouring functions remain + inconsistent — `add_constant_to_itk_image` (line 408) explicitly `.copy()`s a view, while + `divide_itk_images` (line 377) uses two views without copying — but a divide writes to a *new* + output image, so it is not an aliasing bug. +- **Action**: nothing here. The reporters said they would open the PR; upstream can close it. +- **Related**: — + +### U-006 — Physics/performance claims: one dismissed, two resolved via upstream analysis +- **Status**: #1115 **NOT REPRODUCED**; #1135 **explained & fixed on master**; #1143 **confirmed present, cause identified** +- **Category**: #1115 not a defect; #1135 a real race, **already fixed in this tree**; #1143 a real + design issue, **still present** +- **Upstream**: [#1115](https://github.com/OpenGATE/opengate/issues/1115), + [#1135](https://github.com/OpenGATE/opengate/issues/1135), + [#1143](https://github.com/OpenGATE/opengate/issues/1143) +- **#1115 — what I ran**: `ShieldingLIQMD_HP_EMZ` (and `ShieldingLIQMD_HP`, `Shielding_HP_EMZ`, + `Shielding_EMZ`) as `sim.physics_manager.physics_list_name`, then a full 1-thread run + (20 mm air world, 1 MeV gammas, 1000 primaries) on **GATE 10.1.1 + Geant4 11.4.2**. All four + configured, initialized and ran to completion; the baseline `G4EmStandardPhysics_option4` behaved + identically. **Not reproduced** on the pinned Geant4. +- **#1135 — resolved by reading the issue thread, not by re-running it**: the maintainers' comments + identify the cause as **thread races in the TLE bookkeeping** (`GateTLEDoseActor` accumulating + per-thread partial sums), producing rare *upward* spikes in individual voxels while the co-scored + `DoseActor` (a different accumulation path) stays clean. It was **fixed on `master`** by commit + `a92a62bf1` (“fix thread race issue”). +- **#1135 — verified in this tree**: `git cat-file -t a92a62bf1` → commit, and + `git merge-base --is-ancestor a92a62bf1 HEAD` → **true**, so the fix **is present here** + (64 commits after it). The fix is real and inspectable: the racy shared + `fLastEnergy`/`fLastMu` members were replaced by **`thread_local` caches** — see + `core/opengate_core/opengate_lib/GateMaterialMuHandler.cpp:69-71` + (`thread_local … lastHandler / lastCouple / lastTable`). +- **#1135 — the caveat that matters most**: `git tag --contains a92a62bf1` returns **nothing**; + the newest release tag is `10.1.1` and it does **not** contain the fix. So “I am on 10.1.1 and + still see the spikes” is expected, and the issue must **not** be closed as “fixed in the latest + release” — it is fixed **on master only**. This also means re-running it *here* would have been + misleading: this tree is `master`+64, so it would have looked clean and I would have wrongly +dismissed a real bug. **The lesson: a claim about a released version cannot be tested on a tree + that is ahead of that release** (see + [`engineering/simulation-debugger.md`](../engineering/simulation-debugger.md) §5.1). +- **#1143 — confirmed present, mechanism read in the code**: the comments describe serial ROOT + write-out in MT. Verified in the tree: + - `GateDigiCollectionsRootManager.cpp:41-43` calls `ram->SetNtupleMerging(true)` in MT mode, + with the adjacent comment (lines 30-32) that `SetBasketEntries`/`SetBasketSize` + “does not seem to work (default is 4000)” — the small basket size is what makes the mutex + contention hot; + - the singleton is created **without a lock** (`if (fInstance == nullptr) fInstance = new …`, + line 18) — a genuine lazy-init race; + - `GateDigiCollection.cpp:125` calls `am->AddNtupleRow(fTupleId)` **per row**, so every hit + crosses the shared analysis-manager lock; + - `opengate/coordinators.py:121` (`RootMergeCoordinator`) merges split-job outputs and is **not** + a solution for the per-thread `_nt_` files. + `git log -L` shows the `SetNtupleMerging` call dates from **2023** (`d19e408f2`), i.e. this is a + long-standing design choice, not a regression. +- **Action**: #1135 needs no code work here — it is fixed on master; the useful action is to + **say so upstream and note that no release contains it** (a backport/release decision for the + maintainers). #1143 is a genuine open defect: it needs its own branch, a benchmark measuring + write-out scaling against thread count, and a fix (raise the basket size / batch rows per event + instead of per row / guard the singleton). Neither is fixed in this change. +- **Related**: B-001 (the `PhysicsListBuilder registry differs from the linked Geant4` warning), + [`engineering/geant4-physics-expert.md`](../engineering/geant4-physics-expert.md). + +### U-010 — `> 2^31` primaries: **already fixed in the tree** +- **Status**: **fixed** — the crash was replaced by an explicit limit, a warning and a test +- **Category**: repository bug (already resolved) +- **Upstream**: [#1059](https://github.com/OpenGATE/opengate/issues/1059) +- **What the report said**: `GateSourceManager.cpp` emitted `/run/beamOn INT32_MAX` with + `fMaxPrimariesPerRun = INT32_MAX`, so more than `2^31` primaries in one run overflow the `G4int` + taken by `G4RunManager::BeamOn` and the process dies *mid-simulation*. +- **Checked in the tree**: the limit is now **platform-aware** — + `g4.GateSourceManager.GetPlatformMaxPrimariesPerRun()` (`opengate/engines.py:71`) — and reaching + it triggers `GateSourceManager::WarnPrimaryLimitReached()` + (`core/opengate_core/opengate_lib/GateSourceManager.cpp:105`), which emits a `JustWarning` + G4Exception explaining that the run is being stopped **before** generating more primaries instead + of aborting. There is a `max_primaries_per_run` user option, documentation in + `user_guide_sources.rst`, and a regression test. +- **Verified**: `opengate_tests -t source/test098_stop_simulation_at_max_primaries.py` → + `Summary pass: 1/1`, `True` (run on this machine, commit `996572b64`). +- **Action**: none. Upstream can close #1059. +- **Related**: U-003 (the other C++ defects, still open). + +### U-007 — PET crosstalk: feature gap, not a bug +- **Status**: open upstream, **confirmed absent from the Python API** +- **Category**: feature request (explicitly *not* a bug — do not log it as one) +- **Upstream**: [#1145](https://github.com/OpenGATE/opengate/issues/1145) (opened 2026-09-23) +- **Content**: GATE 9's C++ digitizer has a PET optical-crosstalk model; the reporter asks whether + the Python API exposes it, or whether `DigitizerSpatialBlurringActor` can substitute (they + suspect not, for discrete crystals). +- **Verdict**: this is a *missing capability*, and the honest answer is “not implemented in the + Python layer”. Nothing to fix in this triage; if the user wants it, it is a new actor/feature + piece, not a defect. Related open upstream items: `#885` (ComptonCameraActor request), `#877`. +- **Related**: [`engineering/gate-physics-expert.md`](../engineering/gate-physics-expert.md) + +### U-008 — Upstream issues that are environment problems, not code defects +- **Status**: open upstream — **do not “fix” the repo for these** +- **Category**: environment issue / documentation gap +- **Upstream**: [#942](https://github.com/OpenGATE/opengate/issues/942) and + [#938](https://github.com/OpenGATE/opengate/issues/938) (both: `Cannot import opengate_core`, + `libQt6Core-*.so.6.6.2: cannot open shared object file`, and reinstalling with `[novis]` does not + help), [#1000](https://github.com/OpenGATE/opengate/issues/1000) (building ITK 5.2.1 fails on + GCC 15 — `-include cstdint` / `-std=gnu11` needed), [#1141](https://github.com/OpenGATE/opengate/issues/1141) + (“Geant4 version is not ok”). +- **Why they are not repository bugs**: #942/#938 are a *wheel/install* problem — a Qt6 runtime + dependency of the installed wheel is missing on the user's machine, so the C++ module cannot be + dlopen'd; the correct response is to install the Qt6 runtime (or use the `novis` extra + *correctly*), not to change the sources. #1000 is a toolchain-too-new problem with a proven + workaround. #1141 is **exactly this log's B-004/B-005**: the reporter's Geant4 is + `geant4-11-04-patch-02` while the runner prints `Required: v11.4.0` and then `11 4 2` on the next + line — a self-contradicting message produced by B-005's broken `get_required_g4_version()`. +- **Action**: keep the environment guidance in + [`environment-setup/geant4-itk.md`](../environment-setup/geant4-itk.md) correct; nothing here + needs a code change except B-005 (already logged, fix prepared). The ITK/GCC-15 flags are worth + a one-line note in that same skill if the user builds ITK from source. +- **Related**: B-004, B-005, [`environment-setup/geant4-itk.md`](../environment-setup/geant4-itk.md) + +### U-009 — Upstream test-quality issues worth remembering when writing tests +- **Status**: open upstream — neither fixed here +- **Category**: repository bug (tests) +- **Upstream**: [#844](https://github.com/OpenGATE/opengate/issues/844) + (`test075_siemens_cios_alpha.py` “does not test anything”, plus companion + [#885](https://github.com/OpenGATE/opengate/issues/885) “test075… and spekpy issue”) and + [#933](https://github.com/OpenGATE/opengate/issues/933) (reported as flaky; the reporter's own + edit concludes it is *not* flakiness but a plain failure, and that the test is **not in the Linux + CI set**). +- **Why it matters**: both are exactly the failure modes + [`engineering/test-writer.md`](../engineering/test-writer.md) exists to prevent — a test that + asserts nothing, and a test that passes only because CI never runs it. If you touch the SPECT + contrib models or the PBS source, check whether these two files are in the run set. +- **Related**: [`engineering/test-writer.md`](../engineering/test-writer.md), [#993](https://github.com/OpenGATE/opengate/issues/993) + (“Check if actor output is correctly handled in all voxel deposit actors”). + +### B-007 — `core/setup.py` hardcoded a build parallelism of 4 +- **Status**: **fixed in this branch** (uncommitted at time of writing) +- **Category**: repository bug +- **Severity**: medium (build time, not correctness) +- **Area**: `core/setup.py`, `CMakeBuild.build_extension()` +- **Symptom**: `opengate_core` never compiled with more than 4 jobs, so a cold build took + many minutes even on a 16+ core machine; `make -j $(nproc)` in the build directory was + visibly faster than the supported `pip install -e .` path. +- **Cause**: `build_args += ["--", "-j4"]` (Unix) and `build_args += ["--parallel", "4"]` + (Windows, non-Visual-Studio generator). +- **Fix**: default to `os.cpu_count()`, overridable with `OPEN_GATE_BUILD_JOBS`. Both the Unix + and Windows branches now use the computed value. +- **Verification**: the value is computed at build time; re-running a build should now spawn + `os.cpu_count()` compile processes. Confirm with `nproc` against the number of `cc1plus` + processes during a build. +- **Related**: T-018, and `skills/environment-setup` §4.3. + +--- + +## Closed + +| id | summary | resolution | commit | +| --- | --- | --- | --- | +| B-010 | Skills indexes linked to a non-existent `skills/build-and-ci/SKILL.md`. | Wrote `skills/build-and-ci/SKILL.md` (grounded in the real `main.yml` job names and pins); all references now resolve. Cause was B-011. Supervisor note: T-028 subsequently merged that skill into `environment-setup` §6 and removed the directory, so B-010's fix was itself superseded — no dangling reference remains. | uncommitted on `agentic-skills` (T-026, superseded by T-028) | +| B-011 | `.gitignore`'s `build*/` silently ignored `skills/build-and-ci/`, making the B-010 fix un-committable. | Added `!skills/build*-*/`; skill now tracked, real build/dist/output paths still ignored. The rule is kept even though the directory is gone — it prevents the trap recurring for any future `build*`-named skill. | uncommitted on `agentic-skills` (T-026) | + +--- + +## Upstream issue triage (how this file relates to GitHub) + +The rule in the header stands: **things already tracked upstream belong in GitHub issues — link +them, do not duplicate their content.** This section records only *triage*, i.e. what an agent +verified against the local tree, so the next agent does not re-open the same investigation. + +- Source of truth: (112 open at the time of this + triage, 2026). +- Every entry below names the upstream number, the **verdict**, and *what was actually checked*. +- **A verdict is one of three, and they are not interchangeable:** + - **NOT REPRODUCED** — a controlled reproduction was run here and did *not* show the reported + behaviour. This is a real, if negative, result. + - **NOT VERIFIED** — nobody reproduced it in this environment; it is a lead, not a finding. + Never quote it as confirmed. + - **CONFIRMED** — read in the code and/or reproduced; a defect. +- **Two rules that decide whether a claim can be tested here at all:** + 1. **Read the issue's comments before re-running anything.** Maintainers often post the root + cause and the fixing commit; re-deriving it wastes a session (#1135 and #1143 were both + fully diagnosed in their threads). + 2. **A claim about a *released* version cannot be tested on a tree ahead of that release.** + #1135 is fixed on `master` but in no tag, so a run in this tree looks clean and would wrongly + dismiss a real bug. Check `git tag --contains ` before concluding anything. +- **Every reproduction must be validated against a known answer first** — see + [`engineering/simulation-debugger.md`](../engineering/simulation-debugger.md) §5.1. A harness that + gives 0 % transmission through air cannot measure a 25 % difference, and reporting its output as + a confirmed bug is the most expensive mistake in this file. +- When an upstream issue is fixed in a release, move its row out of this section rather than + silently deleting it (the “already fixed” rows below are the reason a triage section exists at + all: **five** of the newest-looking issues were already dead). + +### The short version +| Bucket | Upstream | What to do | +| --- | --- | --- | +| **NOT REPRODUCED** — controlled run says no | #1107, #857, #1035, #1115 | report back upstream; do not fix | +| Already fixed in the tree, still open upstream | #1104, #849, #1059, #616 | nothing here; upstream can close them | +| **Fixed on `master` but in NO release tag** | #1135 | tell upstream; backport is a release decision (U-006) | +| Confirmed present in the tree, not fixed here | #1096, #800, #1032, #1143 | one branch each, test first (U-003, U-006) | +| Confirmed **absent** (missing feature, not a bug) | #1145 | not a defect (U-007) | +| Environment / toolchain, not the repo | #942, #938, #1000, #1141 | fix the machine or the docs (U-008) | +| Test-quality debt | #844, #933 | relevant when you write tests (U-009) | + +--- + +## Common false positives (check before logging) + +Distinguish these from real bugs; they waste the most time: + +| Symptom | Likely cause | Check | +| --- | --- | --- | +| Import error for `opengate_core` | wrong venv / missing local build | `skills/environment-setup` §7 | +| Import error for `opengate_core` mentioning `libQt6Core-*.so` | Qt6 runtime missing for the installed wheel (upstream #938/#942) | U-008 — install Qt6 or use the correct no-visu install; do **not** patch the sources | +| `Geant4 version is not ok` printed by `opengate_tests` | local Geant4 is behind the pin (**or** the B-005 mis-parse) | B-004, B-005 | +| A bug report you cannot find in the code any more | it may already be fixed though the issue is still open upstream | U-001 | +| Many tests failing on missing input files | `opengate/tests/data` submodule not initialized **or behind the recorded pointer** (B-008/B-009) | `ls opengate/tests/data`; then `git submodule update --init --recursive` and compare `git -C opengate/tests/data rev-parse HEAD` with `git ls-tree HEAD opengate/tests/data` | +| Import error for `torch` / `gaga_phsp` / `pytomography` | optional extras not installed | `skills/environment-setup` §5 | +| `RuntimeError: Numpy is not available` (or `A module that was compiled using NumPy 1.x…`) in a GAN/`garf` test — torch imports fine, plain tensors work, but `.numpy()` / `from_numpy()` fails | `torch`/`numpy` **ABI mismatch** for this platform (B-012) | `skills/environment-setup` §5.2 — find the set by search for *this* platform; the versions are **not** portable | +| `cannot allocate memory in static TLS block` | Geant4 TLS model | `skills/environment-setup` §4.5 | +| `ImportError: dlopen(…/opengate_core…so): Library not loaded: /usr/local/opt/qtbase/…/QtWidgets` on **Intel** macOS | Homebrew dependencies gone — no Intel support; Geant4, ITK and `opengate_core` all have stale `/usr/local` paths | `otool -L` the failing lib; rebuild against MacPorts per `skills/environment-setup/geant4-itk.md` §2 (macOS **Intel**) — **do not** install the `opengate-core` wheel instead. On Apple Silicon swap `/usr/local` → `/opt/homebrew` and use `brew` — **not** this row | +| `ld: library 'fftw3_threads' not found` after a dep swap | ITK built with `ITK_USE_SYSTEM_FFTW=ON` against the old prefix | reconfigure ITK with MacPorts (or the current) FFTW lib paths — `skills/environment-setup/geant4-itk.md` §2 (macOS) | +| Test passes alone, fails in the suite | resource contention (`-n all`) or test interdependence | rerun with `-p sp -n 1`, then with `-f` | +| Stochastic assertion failing at low statistics | test design, not code | inspect the tolerance, not the simulation | +| Behaviour differs from the docs | docs may lag the code | hierarchy of truth in `AGENTS.md` §5 | diff --git a/skills/status/project-status.md b/skills/status/project-status.md new file mode 100644 index 000000000..9cb83226b --- /dev/null +++ b/skills/status/project-status.md @@ -0,0 +1,209 @@ +# Project status snapshot + +Rolling snapshot of what this repository *is* and in what state it is, so an agent can +orient itself without re-reading the tree. **Date every observation** — this file rots +faster than the code. + +--- + +## Snapshot + +- **Date of last update**: 2026-09-22 +- **Version** (`VERSION`): 10.1.1 +- **Python**: requires >= 3.9 in `pyproject.toml`; CI builds and tests **3.10 → 3.14** + (`requires-python` and CI matrices disagree slightly — trust CI for what is exercised). +- **Native stack**: Geant4 `v11.4.2`, ITK `v5.4.4` (pinned in + `.github/workflows/main.yml`). The developer guide still mentions ITK `v5.2.1` — stale. +- **Packages**: `opengate` (Python) + `opengate_core` (C++), the latter hard-pinned as + `opengate-core==` in `setup.py`. +- **Platforms tested in CI**: `ubuntu-24.04` (+ `ubuntu-24.04-arm` on tag/schedule), + `macos-15` (+ `macos-15-intel` on tag/schedule), `windows-2025`. +- **Known platform limitation**: on Windows, multithreading and Qt visualization are not + available (`README.md`). + +## Local reference environment (verified) + +A working dev environment was exercised against this branch, so the following is measured, +not assumed. **Machine-specific absolute paths are not recorded here** — they belong in the +(sourced, git-ignored) `user_secrets.json`; see `skills/environment-setup` §0. + +- **`user_secrets.json` at the repo root** holds `OPEN_GATE_REPO`, `OPEN_GATE_ENV`, + `OPEN_GATE_DEPS`. It is git-ignored + (`.gitignore` → "LOCAL MACHINE CONFIG"), so it is present on a developer's machine and + absent from a fresh clone — agents must create it and ask the user for the values. +- **Environment type actually verified**: a **`uv`-created** venv (uv 0.12.10), CPython + 3.14.7. Its `pyvenv.cfg` recorded `version_info = 3.14.4` while the interpreter reported + 3.14.7; the runtime wins. +- **No `pip`** in a `uv` venv (`No module named pip`, no `bin/pip`): use `uv pip` with + `VIRTUAL_ENV="$OPEN_GATE_ENV"`, or plain `python -m pip` on a stdlib venv. +- **Both packages editable** at `$OPEN_GATE_REPO` and `$OPEN_GATE_REPO/core`. +- **Geant4 now matches the pin**: the checkout was originally at `v11.4.0` (runner warned + `Geant4 version is not ok`); it was updated to tag `v11.4.2`, rebuilt, and `opengate_core` + relinked. The runner now reports **`Geant4 version is OK`** + (`geant4-11-04-patch-02`, `G4VERSION_NUMBER 1142`). This was an environment issue (B-004), + not a repository bug. +- **Geant4/ITK builds were reused, not rebuilt from scratch**: two pre-existing build trees + under `$OPEN_GATE_DEPS`, which `core/config.json` does not pin (the file is absent, so + `setup.py` passes empty `Geant4_DIR`/`ITK_DIR` and CMake resolves them via + `CMAKE_PREFIX_PATH`). The from-scratch source build (`skills/environment-setup` §4.1–4.2) + was therefore **not** exercised (T-003). +- **Geant4 data** downloads on the first `import opengate_core` into + `core/opengate_core/geant4_data/` (6 archives, several minutes of apparent silence — + not a hang). + +## Build / test pipeline health + +- CI entry point: `.github/workflows/main.yml` (push to `master`, PRs to `master`, tags, + cron `0 0 * * 0,3`, manual dispatch). +- Wheel build: `.github/workflows/actions_build/action.yml` + three OS scripts; caches + Geant4/ITK under `~/software`. +- Test run: `.github/workflows/actions_tests/action.yml` — installs the **built** wheels + then runs `opengate_tests -r -s ` (seeded random subset + last 10 tests). +- Results history: (aggregated from + the uploaded `results_json-*` artifacts into `OpenGATE/opengate_tests_results`). +- Docs: Read the Docs builds from `docs/`; CI does not run on `docs/**` changes. + +*Fill in the last observed CI status when you touch this file* (green/red per OS, link to +the run). + +## Last observed local run + +Not CI — a local run on the reference environment above, branch `agentic-skills`, after +rebuilding `opengate_core` (root cause in B-001): + +### Full-suite baseline (368 tests, ~17.6 min) + +Command: `opengate_tests` (no filters), commit `8f049324`, log kept outside the repo in a +non-volatile scratch dir. Result: **366/368 passed, 2 failed**. + +| Metric | Value | +| --- | --- | +| Tests available / run / ignored | 458 / **368** / 90 | +| Passed | **366** | +| Failed | **2** | +| Dependency split | 354 in round 1, then 14 with mutual dependencies | +| Wall time | 17.6 min | +| Final summary line | `False` (i.e. not all passed) | +| Per-test logs | `opengate/tests/log/*.log` | + +Failures: + +| Test | Time | Note | +| --- | --- | --- | +| `geometry/test102_gammex467.py` | 44.1 s | geometry; plausibly Geant4-version sensitive | +| `geometry/test107_macaco1_mt.py` | 142.5 s | geometry + multithreaded | + +**Validity caveat — now resolved.** This baseline was taken with **two** environment faults, +neither of which was a repository bug: + +1. **Geant4 11.4.0** instead of the pinned 11.4.2 (B-004) — since fixed by rebuilding Geant4 + at v11.4.2 and relinking `opengate_core`; the runner now prints `Geant4 version is OK`. +2. **A stale `opengate/tests/data` submodule**, four commits behind the pointer recorded in + this repo (B-009) — since fixed by `git submodule update --init`; the submodule is now at + `9fabbdddf`. + +**Both baseline failures were caused by fault 2 and are now green.** Re-run on the corrected +environment (Geant4 11.4.2, correct submodule): + +``` +Running: geometry/test102_gammex467.py OK 18.1 s +Running: geometry/test107_macaco1_mt.py OK 60.6 s +Summary pass: 2/2 passed the tests +True +``` + +Both logs end with “Great, tests are ok.”, including test107's physics assertion (FWHM within +tolerance). **No repository bug was found.** So the 366/368 baseline should be read as an +environment artifact, and a fresh full-suite run on the corrected tree is expected to be +green (T-014). + +### Targeted verification runs + +| Command | Result | +| --- | --- | +| `python -c "import opengate, opengate_core"` | clean; `opengate_core` resolves to `$OPEN_GATE_REPO/core/opengate_core` | +| `opengate_tests -t actors/test008_dose_actor.py` | **1/1 passed**, `True`, ~0.3 min | +| `opengate_tests -i 1 -e 12 -n 4` | **30/30 passed**, `True`, 2.2 min | +| `opengate_tests -l` | `114` (highest test id) | + +The runner auto-runs `misc/test001_g4threevector.py` first to trigger data download, so +single-test runs report a 2-test progression (`30` for `-i 1 -e 12` includes it). +Two prerequisites were required and are easy to get wrong — the venv must be **activated** +(B-002) and `opengate_core` must be **freshly compiled** (B-001). + +Not exercised locally: the torch / `gaga_phsp` / `pytomography` tests (T-009), Windows/macOS, +and the from-scratch source build (T-003). + +## Test suite shape + +- Location: `opengate/tests/src/{actors,advanced_tests,chemistry,external,geometry,misc,physics,source}`. +- Runner: `opengate_tests` (see `skills/running-tests`). Not pytest. +- **Size (measured)**: 458 `testNNN*.py` files; the runner reports **368 to run and 90 + ignored** (ignore patterns: `_helpers`, `wip`, `debug`, `> `, …), plus 11 skipped for + missing `torch`. Of the 368 available, a 30-test slice passed **30/30**. +- Binary test data: `opengate/tests/data` submodule on gitlab.in2p3.fr (git-lfs), + **75 entries** when populated. +- Optional-dependency tests: some need `torch`, `gaga-phsp`, `garf`, `pytomography`, + `hist` — CI installs them; the reference env does not. +- Per-test logs: `opengate/tests/log/.log` (the fastest place to read a failure). +- Dashboard JSON: `opengate/tests/output_dashboard/dashboard_output__..json` + — e.g. `dashboard_output_linux_3.14.json` on the reference env. Values are `[""]` until + the test has run; that empty status is what `opengate_tests -f` uses to re-run failures. +- **The runner does not validate the environment**: `check_environment()` + (`opengate/bin/opengate_tests_helpers.py`) only warns on a Geant4 version mismatch and + checks the data folder. A broken `opengate_core` (B-001) or an un-activated venv (B-002) + produces hundreds of identical failures instead of one clear error. + +## Documentation state + +- Sphinx sources under `docs/source/{user_guide,developer_guide}`; themed with PDJ. +- `user_guide_reference_*.rst` = exhaustive references; `user_guide_*.rst` = narratives. +- Known stale spots: `DesignGuidelines.md` (marked **OBSOLETE**), the ITK version in + `developer_guide_installation.rst`, and `opengate/tests/readme.md` (says + "documentation in progress"). + +## Agent-facing assets (this directory) + +| Asset | Purpose | +| --- | --- | +| `AGENTS.md` | entry point, golden rules, skills index | +| `user_secrets.json` (root, **git-ignored**) | per-machine paths: `OPEN_GATE_REPO`, `OPEN_GATE_ENV`, `OPEN_GATE_DEPS` | +| `skills/README.md` | map of the skills directory, role-vs-task split | +| `skills/environment-setup` | dev environment (index: §0–§3, §5, §7–§9) | +| `skills/environment-setup/geant4-itk.md` | build Geant4/ITK and rebuild the `opengate_core` extension | +| `skills/environment-setup/build-and-ci.md` | packaging, releasing, CI topology, reproduce CI locally | +| `skills/running-tests` | the `opengate_tests` runner, filters, dashboard | +| `skills/code-style` | pre-commit / black / clang-format, conventions | +| `skills/architecture` | managers/engines/actors/sources lifecycle | +| `skills/documentation` | Sphinx pages and local build | +| `skills/engineering/SKILL.md` | index: which role to pick, the two layered pairs, conventions | +| `skills/engineering/python-gate-developer` | role: Python API design, C++ boundary, serialisation | +| `skills/engineering/geant4-physics-expert` | role: Geant4 physics — lists, EM models, cuts, stats | +| `skills/engineering/gate-physics-expert` | role: GATE physics layer — physics manager, sources, actors, normalisation | +| `skills/engineering/gate-chemistry-expert` | role: GATE chemistry layer — Geant4-DNA chemistry lists, chemistry world, chemistry actors/counters | +| `skills/engineering/geant4-geometry-expert` | role: raw G4 geometry — solids, volumes, navigation, overlaps | +| `skills/engineering/gate-geometry-expert` | role: GATE geometry layer — volume tree, solids, materials, regions | +| `skills/engineering/simulation-debugger` | role: triage, reproduction, lifecycle-localised diagnosis | +| `skills/engineering/test-writer` | role: test placement, assertions, tolerances | +| `skills/status/` | this snapshot, `task-list.md`, `found-bugs.md` | + +## Open questions / risks to watch + +- The skill documents describe the tree at the commit they were written against; CI keys + and runner options drift. If a command in `skills/` fails, fix the skill as part of the + same change (`AGENTS.md` §5). +- `user_secrets.json` is git-ignored, so a fresh clone **has no paths at all**: the first + agent on a machine must create it and ask the user. Treat "file missing" as normal, and + treat "committed" as a security incident (remove from the index immediately). +- `requires-python = ">=3.9"` vs. CI floor 3.10 vs. `README.md` claiming 3.10+: keep an + eye on which one the packaging actually honours. +- The `docs/**` path filter means documentation regressions are only caught by Read the + Docs builds, not by the main CI. + +## Update protocol + +1. Set the date line when you edit. +2. Change **facts**, not opinions; keep each line verifiable from a file or a CI run. +3. If a statement stops being true, delete it rather than leaving a stale note. +4. Log anything you discover but do not fix in [`found-bugs.md`](found-bugs.md) and + [`task-list.md`](task-list.md). \ No newline at end of file diff --git a/skills/status/task-list.md b/skills/status/task-list.md new file mode 100644 index 000000000..49ea7d978 --- /dev/null +++ b/skills/status/task-list.md @@ -0,0 +1,81 @@ +# Task list (agent-maintained) + +Shared work list for agents and humans on this repository. **Update it in the same turn +you start or finish work** — an agent that plans work here but does not mark it done makes +the next agent repeat it. + +Rules: + +- One line per task, with a stable id (`T-001`, `T-002`, …). +- Status is one of: `todo`, `doing`, `blocked`, `done`, `dropped`. +- Always name the files/areas touched, so the next reader can find the change. +- When a task reveals a bug you are not fixing now, add it to + [`found-bugs.md`](found-bugs.md) and link the bug id here. +- Keep `done` tasks for one release cycle, then move them to the archive at the bottom. + +Columns: **id · status · area · task · evidence**. + +--- + +## Active + +| id | status | area | task | evidence | +| --- | --- | --- | --- | --- | +| T-001 | done | `AGENTS.md`, `skills/` | Add agent entry point + skills for environment setup, tests, build/CI, code style, architecture, docs, and a status folder. | Files added on branch `agentic-skills`; see this directory. | +| T-005 | done | `skills/*` | First trial of the skills against a real environment: import check, smoke test, 30-test slice. | `opengate_tests -i 1 -e 12 -n 4` → `Summary pass: 30/30`, `True`, 2.2 min; `-t actors/test008_dose_actor.py` → `1/1`, `True`. Paths came from `user_secrets.json`, not hard-coded. | +| T-006 | done | `skills/environment-setup`, `skills/running-tests`, `AGENTS.md` | Fix defects the trial exposed: wrong `-t` example path, missing venv-activation requirement, `pip` vs `uv` on uv-made venvs, stale-`.so` rebuild procedure. | Same commit; each correction cites the observed failure. | +| T-010 | done | `skills/*`, `AGENTS.md`, `.gitignore` | Move all machine-specific paths into a git-ignored `user_secrets.json` at the repo root; make every skill command read from it instead of hard-coding paths. | `git check-ignore -v user_secrets.json` → `.gitignore:88`; no absolute dev path remains in `AGENTS.md` or `skills/*`. | +| T-002 | todo | `skills/` | Keep skills in sync with the code: any PR that changes test runner options, CI keys, or the object lifecycle must update the matching `SKILL.md`. | Proven necessary by T-006 — the trial found 3 wrong instructions. | +| T-029 | todo | `opengate/bin/opengate_tests_helpers.py` | **Apply the two verified runner patches** that stage scoping left uncommitted: B-005 (read `env.GEANT4_VERSION` from the workflow level; also guard `tests_dir.parents[2]` against `IndexError`) and B-002 (launch tests with `sys.executable` via `subprocess.run([...], shell=False)`). | Both patches were written and **proved at runtime** this session, then reverted because only `skills/`/`AGENTS.md` were in scope. Repro + proof recorded in `found-bugs.md` B-002/B-005. | +| T-003 | todo | `skills/environment-setup` | Verify the *full* source-build path (§4.1–4.4) end to end, not just the reuse of an existing build tree as in T-005. | Only the rebuild against pre-existing Geant4/ITK builds was exercised. | +| T-004 | todo | ops | Decide how generated session logs (`opengate/tests/log/`, `output_dashboard/`) are shared between agents without being committed. | — | +| T-007 | todo | `opengate/bin/opengate_tests_helpers.py` | The runner hardcodes `python ` instead of `sys.executable`, so an un-activated venv fails every test opaquely. Consider using `sys.executable` (a `FIXME` already exists at line ~341). | See B-002. Needs a Windows-safe quoting approach per that comment. | +| T-008 | todo | `opengate/bin/opengate_tests_helpers.py` | `check_environment()` does not detect a stale `opengate_core`; the suite reports 458 tests discovered and then every one fails. A version/`hasattr` preflight would fail fast. | See B-001. | +| T-011 | done | `skills/*`, `AGENTS.md` | Verify no absolute dev-machine path survives in any committed file. | **Verified clean.** `grep -rnE '/Users/[a-zA-Z]\|/home/[a-zA-Z]\|/Data/\|C:\\\\Users' AGENTS.md skills/` returns only the audit instruction itself (`task-list.md`). All remaining paths are placeholders (`/absolute/…`, ``, ``); machine values live in the git-ignored `user_secrets.json`. | +| T-013 | todo | `opengate/bin/opengate_tests_helpers.py` | `get_required_g4_version()` indexes the non-existent `jobs.build_wheel` and silently falls back to a literal; read the workflow-level `env.GEANT4_VERSION` instead. | See B-005. | +| T-014 | done | tests | Re-run the two baseline failures on the corrected environment and confirm whether they are genuine repository bugs. | **`geometry/test102_gammex467.py` OK (18.1 s)** and **`geometry/test107_macaco1_mt.py` OK (60.6 s)** → `Summary pass: 2/2`, `True`; logs end with “Great, tests are ok.” **No repository bug** — the failures came from a stale `opengate/tests/data` submodule checkout (T-020). | +| T-020 | done | submodule / tests | Fix the `opengate/tests/data` checkout, which lagged the pointer recorded in the parent repo by four commits, so the reference data for test102/test107 was absent. | `git submodule update --init opengate/tests/data` → HEAD `9fabbdddf`; `output_ref/test102_gammex467/` and `output_ref/test107_macaco1/` now populated. See B-008. | +| T-021 | todo | `opengate/tests/utility.py` | `create_output_ref()` calls `mkdir(..., exist_ok=True)`, so a missing reference dataset produces an *empty directory* instead of a clear “reference data missing” error — the test then fails deep inside the comparison. Consider asserting the expected files exist. | See B-008. | +| T-019 | todo | `core/setup.py` | Consider mirroring the parallelism fix into a documented env var for CI, and check whether `core/config.json` should be auto-created from `CMAKE_PREFIX_PATH` so the Geant4/ITK wiring is explicit. | See `skills/environment-setup` §4.3 and B-007. | +| T-015 | todo | tests / `.gitignore` | A full-suite run leaves an untracked `simulation.json` in the repo root; make the test write to an ignored path or add an ignore rule. | See B-006. | +| T-016 | done | `AGENTS.md` | Encode the git safety rules: `master` is read-only for agents (sync only), every change on a fresh branch cut from the up-to-date local `master`, never commit without user approval. | `AGENTS.md` golden rules 1–2 and new §6 (sync, branch creation, rebase policy). | +| T-017 | done | environment | Relink `opengate_core` against the rebuilt Geant4 v11.4.2 so the suite is CI-comparable. | Relink via `uv pip` succeeded; runner now prints `Geant4 version is OK`. First attempt failed (`python -m pip` on a `uv` venv). | +| T-018 | done | `core/setup.py` | Remove the hardcoded `-j4` / `--parallel 4` and default to all cores, with an `OPEN_GATE_BUILD_JOBS` override. | See B-007. | +| T-022 | done | `skills/README.md`, `skills/engineering/` | Add a skills README (map of the directory, role-vs-task split, conventions, how to add a skill) and four role-based engineering skills: `python-gate-developer.md`, `geant4-physics-expert.md`, `simulation-debugger.md`, `test-writer.md`. | `skills/README.md` added; `skills/engineering/` created; both listed in the `AGENTS.md` skills index and the `project-status.md` asset table. | +| T-023 | done | `skills/engineering/` | Add the missing role skills: `gate-physics-expert.md` (GATE physics layer — physics manager, **sources**, **actors**, filters, biasing, normalisation), `geant4-geometry-expert.md` (raw G4 geometry — solids, volumes, navigation, overlaps, C++ bindings) and `gate-geometry-expert.md` (GATE geometry layer — volume tree, solids, materials, placement/repetition, regions, fields). | Three new files in `skills/engineering/`; each grounded in real class names (`SolidBase`/`BoxSolid`…, `GenericSource`, `DoseActor`, `reference_physics_list_*`); listed in `skills/README.md`, the `AGENTS.md` skills index and the `project-status.md` asset table. | +| T-024 | done | `skills/engineering/SKILL.md` | Add the role-skill index so the `engineering/` directory follows the `/SKILL.md` convention and its roles are discoverable from the top-level indexes. | `skills/engineering/SKILL.md` added (role table, the two layered pairs, role composition, conventions, how-to-add); linked from `skills/README.md` (layout tree + "which skill" table + role section), `AGENTS.md` skills index and `project-status.md`. | +| T-025 | done | environment | Verify the dev environment end to end after the Geant4 11.4.2 relink: 30-test slice. | `opengate_tests -i 1 -e 12 -n 4` → `Summary pass: 30/30`, `Geant4 version is OK`, `True`, 5.9 min (log kept outside the repo, under `$OPEN_GATE_ENV`). Both packages at 10.1.1 == `VERSION`; no untracked pollution in the repo root. | +| T-027 | done | `skills/` | De-duplicate the four skills whose titles look overlapping (`environment-setup`/`build-and-ci`, `code-style`/`architecture`) by giving each fact a single owner and replacing copies with cross-references; document the boundary rules in `skills/README.md`. | `environment-setup` §6 (CI recipe, ~20 lines) reduced to a pointer at `build-and-ci` §5 — `grep -c 'python -m build'` now returns 1, not 2. `build-and-ci` §6 reframed as *packaging consequences only*, pointing at `environment-setup` §4.3. Corrected `build-and-ci` §4 `-r` wording to match the real help text (`Start the last 10 tests and 1/4 of the others randomly`, `opengate/bin/opengate_tests.py:41`). Added an "Easily-confused pairs — one owner per fact" table to `skills/README.md` and a boundary note in `AGENTS.md` §3. All links resolve; pre-commit clean. | +| T-028 | done | `skills/environment-setup/`, `skills/build-and-ci/` (removed) | Merge `build-and-ci` into `environment-setup` §6 and delete the separate directory — the two overlapped in their CI-recipe text and a single "build & environment" skill is easier to keep truthful. Repointed every reference. | `skills/build-and-ci/` removed; `environment-setup` §6 rewritten as *Build, packaging and CI* with subsections 6.1 topology, 6.2 release, 6.3 what CI tests, 6.4 reproduce locally, 6.5 `core/` packaging consequences, 6.6 safe workflow edits. Header of `environment-setup` now states the two-part scope. References updated in `AGENTS.md` §3, `skills/README.md` (tree, which-skill table, boundary table), `skills/engineering/SKILL.md`, `skills/status/project-status.md`. Two relative links corrected for the new directory depth (`../engineering/…`, `../running-tests/…`). Link check over the whole tree: 0 broken. | +| T-026 | done | `skills/build-and-ci/SKILL.md`, `.gitignore` | Solve B-010 by writing the missing build/CI skill instead of dropping the index rows, and fix the hidden cause found while doing it (B-011: `.gitignore`'s `build*/` made the directory un-committable). | All 7 job names grepped and confirmed in `.github/workflows/main.yml`; `setup.py:15` pin, `core/setup.py:30` `VERSION` read, cron `0 0 * * 0,3`, `paths-ignore: docs/**` and tag-only publish each verified. `!skills/build*-*/` added to `.gitignore`: skill now appears in `git status`, while `build/`, `core/build/`, `dist/`, `*.whl` stay ignored. B-010 and B-011 moved to **Closed**. | +| T-030 | done | `skills/engineering/` | Add the missing `gate-chemistry-expert.md` role skill: the GATE chemistry layer (Geant4-DNA chemistry lists, chemistry world and scavengers, track-structure EM regions, chemistry actors and counters), including the alpha-stage warning. | New file `skills/engineering/gate-chemistry-expert.md`, grounded in real names from the code (`ChemistryManager`, `ChemistryList`, `ChemistryWorld`, `ChemicalCountingActor`, `chemistry_counter_types`, `known_g4_chemistry_list_names`, `set_track_structure_em_physics`); `H2O`-not-a-product, integer-`pH`, single-molecule-counter and manager-policy-conflict `fatal`s each cited from the source. Listed in `skills/README.md` (tree + which-skill + role tables), `skills/engineering/SKILL.md` (tables + worked example), the `AGENTS.md` skills index and `project-status.md`. | +| T-031 | done | `skills/environment-setup/` | Apply the "SKILL.md is an index" rule: `environment-setup/SKILL.md` was 862 lines (6× every other skill) and violated it. Extract its two deep topics to dedicated files — `geant4-itk.md` (Geant4/ITK/`opengate_core` compilation) and `build-and-ci.md` (packaging/release/CI) — leaving `SKILL.md` as an index with pointer stubs at §4 and §6. | `SKILL.md` 862 → 502 lines; new `geant4-itk.md` (280) and `build-and-ci.md` (188). Section numbering re-verified contiguous §0–§9; every moved-section ref repointed (`§4.6` → `geant4-itk.md` §7, `§4.5` → §6, `§4.3` → §4) across `AGENTS.md`, `skills/README.md`, `skills/engineering/{SKILL,gate-physics-expert,gate-geometry-expert,gate-chemistry-expert,geant4-geometry-expert,simulation-debugger,python-gate-developer,test-writer}.md` and `project-status.md`. Link scan over the whole tree + `AGENTS.md`: 0 broken. Convention added to `skills/README.md` §"Adding a new skill" (split past ~250 lines). | +| T-032 | done | `skills/status/found-bugs.md`, upstream | Triage all 112 open upstream issues; **verify** the six claims previously left unverified and promote any confirmed one to an active bug. | Verified on commit `996572b64` (GATE 10.1.1, Geant4 11.4.2): #1107 **NOT REPRODUCED** (tubs=500032 vs hex=500026 → ratio **1.0000**, seeds 1/2/3); #857 **NOT REPRODUCED** (repeater innocent — the missing ROOT file was a zero-hit source); #1035 **NOT REPRODUCED** (`End == size`, `uproot` keys `Hits;1`); #1115 **NOT REPRODUCED**; #1059 **already fixed** (`GetPlatformMaxPrimariesPerRun` + warning + `source/test098…` → `1/1`); #616 **already fixed**. #1135 **resolved from the issue thread** — real TLE thread race, fixed on `master` by `a92a62bf1` (`git merge-base --is-ancestor` → true; `thread_local` caches at `GateMaterialMuHandler.cpp:69-71`) but **in no release tag** (`git tag --contains` → empty), so it cannot be dismissed here. #1143 **CONFIRMED present** (unlocked singleton, `SetNtupleMerging(true)` + 4000-entry basket, `AddNtupleRow` per row) → added as active bug U-006. Verdicts in `found-bugs.md` U-002…U-010. | +| T-033 | done | `skills/engineering/` | Record the harness and triage mistakes made while verifying upstream issues, so future sessions do not repeat them. | `gate-physics-expert.md` §4.1–4.2 (beam/source traps, `beam2d`≠parallel, `Run0254 No Default Region`), `gate-geometry-expert.md` §4.1–4.2 (boolean operands must not be placed; Tubs/Hexagon area formulas + circumradius trap), `simulation-debugger.md` §5.1 (harness-vs-repo-bug table) **and §5.2 (read the issue comments first; a claim about a released version cannot be tested on a tree ahead of that release — the #1135 false-dismissal trap)**, `test-writer.md` §7. | +| T-034 | done | environment ($OPEN_GATE_ENV) | Install/fix the optional extras so the dependent tests can run, then run them. | The env had `torch 2.2.2` against `numpy 2.4.4` → every NumPy↔torch bridge call raised `RuntimeError: Numpy is not available`. Fix was **found by search, not hard-coded**: on `macOS-15.8-x86_64` PyTorch ships **no wheel newer than 2.2.2** (Intel-Mac builds were dropped), which forces `numpy<2`; installed `numpy==1.26.4 pandas==2.3.3 scipy==1.12.0 torch==2.2.2`. Also installed the extras the skill had omitted: `pytomography hist pyvista pymedphys`. Results: `test081…` `1/1`; the 7-test GAN/`garf` batch `8/8`; `external/pytomography/test094d_reconstruction.py` `1/1` (`True`, 128.8 s). Runner now discovers **376** files to run (was 368); §5.1 audit prints `NOTHING MISSING`. See B-012. | +| T-035 | todo | `opengate/tests/src/**/*wip*.py` | Decide the fate of the **28 `wip`-flagged test files** currently excluded by the runner's `wip` ignore pattern (`get_ignored_patterns()`, `opengate/bin/opengate_tests_helpers.py`): either finish and rename each to the normal `testNNN_*.py` form so it enters CI, or delete it. They all parse (`ast.parse` OK) but none is executed by CI, so they rot silently. | Inventory (28 `wip` files under `opengate/tests/src/`, i.e. 27 tests + the helper `actors/test043_garf_helpers_wip.py`; counted with `find opengate/tests/src -name '*wip*.py'`): `actors/` 11 — `test043_garf{,_analog,_mt,_training_dataset,_training_dataset_full}_wip`, `test058_calc_uncertainty_mt{,_ste_mean}_wip`, `test058_uncertainty_flags_over_multiple_runs_mt_wip`, `test081_tle_1_geom_mt_wip`, `test081_tle_3_speed_wip`, `test085_free_flight_spect_1_ref_mt_wip`; `external/` 3 — `castor/test096_pet_castor_{coinc,ref}_wip`, `coresi/test097c_coresi_reconstruction_wip`; `geometry/` 1 — `test075_siemens_cios_alpha_wip` (see U-009 / upstream #844); `misc/` 3 — `test024_py_actor_wip`, `test045_speedup_all_wip`, `test045_speedup_wip`; `physics/` 3 — `test013_phys_lists_{2,3,5}_wip`; `source/` 6 — `test021_voxel_source_old_wip`, `test053_phid_0{1,2,3}_wip`, `test056_template_source_wip`, `test060_phsp_source_particletype_fromphs_particlename_wip`. Note 3 more (`test038_gan_phsp_spect_gan_my.py`, `test043_garf.py`, `test043_garf_mt.py`) are listed in `get_torch_dependent_tests()` but **do not exist** in the tree — `test043_garf{,_mt}` exist only as `_wip`, so that list should be corrected too. | +| T-009 | done | tests | Run the optional-dependency tests (torch / gaga_phsp / garf / pytomography). | **Unblocked** by T-034/B-012 — extras installed (`pytomography hist pyvista pymedphys` plus the coherent `torch`/`numpy` set): `test081…` `1/1`, the 7-test GAN/`garf` batch `8/8`, `external/pytomography/test094d_reconstruction.py` `1/1` (`True`, 128.8 s); the §5.1 audit over the whole test tree prints `NOTHING MISSING`. | + +## Blocked + +| id | status | area | task | blocker | +| --- | --- | --- | --- | --- | +| — | — | — | *(nothing blocked)* | — | + +## Archive (completed) + +| id | status | area | task | evidence | +| --- | --- | --- | --- | --- | +| — | — | — | *(nothing archived yet)* | — | + +--- + +## How to use this file without creating noise + +1. Before starting work: search this file for the area you are touching. If a task + exists, switch it to `doing` and add your name/agent id. +2. Do not create a task for a one-line fix that you complete immediately — such work is + visible in git history. +3. `evidence` is mandatory for `done`: the command you ran, the commit hash, or the file + you changed. "Looks fine" is not evidence. +4. If you drop a task, say **why** in the task line rather than deleting it silently.