Repository navigation
[DRAFT] Agentic skills - #1144
Draft
BishopWolf wants to merge 11 commits into
Draft
BishopWolf wants to merge 11 commits into
BishopWolf wants to merge 11 commits into
Conversation
Add an agent entry point (AGENTS.md) plus a skills/ directory covering: - environment-setup: how to build a virtual development environment where all tests run (venv, optional pip-install path, full Geant4/ITK/ opengate_core source build, optional test extras, CI reproduction). - running-tests: the opengate_tests runner, filtering, dashboard output, how to add a test. - build-and-ci: the two-package release flow, wheel builds, CI topology and the rules for editing workflows. - code-style: pre-commit (black/clang-format), naming and import conventions, what must never be committed. - architecture: managers/engines/actors/sources lifecycle, mandatory actor class structure, serialisation/MP constraints. - documentation: Sphinx layout, local build, where to document what. - status/: task list, found-bugs log and a project status snapshot. Also add the skills index to the repo entry point and record the golden rules (submodules first, keep generated artifacts out of git, cite real evidence).
Remove every absolute, machine-specific path from the agent-facing docs and read them from a single per-machine file instead. - Add user_secrets.json (root) holding OPEN_GATE_REPO, OPEN_GATE_ENV and OPEN_GATE_DEPS, plus an explanatory template. It is intentionally a placeholder until the user fills in real values. - Git-ignore user_secrets.json / user_secrets.*.json so it is never committed. - AGENTS.md: new golden rule for the secrets file; quick start now loads the environment path from it instead of hard-coding one. - skills/environment-setup: new section 0 (read-or-create the secrets file, load it into the shell, reuse an existing environment before creating one, create it only on non-volatile storage); every command now uses $OPEN_GATE_ENV / $OPEN_GATE_REPO / $OPEN_GATE_DEPS. - skills/running-tests, skills/status/*: same treatment; drop the recorded absolute paths from the status snapshot and bug reproductions, since those files are committed. - Verified: git check-ignore resolves user_secrets.json to .gitignore:88, the file is absent from git status, and the read-from-JSON -> activate -> import -> smoke test chain passes (1/1).
core/setup.py pinned the CMake build to 4 parallel jobs on every platform
('-j4' on Unix, '--parallel 4' on Windows), so a cold opengate_core build
never used the machine's cores despite taking several minutes.
Default to os.cpu_count() instead, overridable with OPEN_GATE_BUILD_JOBS,
in both the Unix and the Windows branches.
Also update the agent docs to match reality, verified against the toolchain
on this machine:
- environment-setup: document the real build mechanism (setuptools
build_meta + core/setup.py, Geant4_DIR/ITK_DIR from core/config.json
falling back to CMAKE_PREFIX_PATH discovery), the parallelism fix, and
how to make repeat builds incremental with ccache / plain make -j /
ninja / ccmake. Record the 'python -m pip' vs 'uv pip' trap that made a
relink fail silently with 'No module named pip'.
- status/found-bugs: add B-007 (the -j4 defect, fixed here), add a
category column separating repository bugs from environment issues,
reclassify B-004 (Geant4 11.4.0 vs the pinned 11.4.2) as an environment
issue, now resolved by rebuilding Geant4 at v11.4.2 and relinking.
- status/project-status: record the corrected Geant4, the full-suite
baseline (366/368) and its validity caveat.
- status/task-list: keep tasks and evidence in sync (T-012, T-014,
T-017–T-019).
- AGENTS.md: add the git safety rules (master is read-only, always work
on a fresh branch cut from the up-to-date master) and broaden the golden
rules for test-artifact pollution and environment-vs-repo-bug triage.
Re-ran the two failing geometry tests on the corrected environment (Geant4 11.4.2 + relinked opengate_core). Both pass: 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 Root cause of the failures: the opengate/tests/data submodule was checked out four commits behind the pointer recorded in this repo, so the reference datasets (output_ref/test102_gammex467, output_ref/test107_macaco1) were absent. After 'git submodule update --init opengate/tests/data' (HEAD now 9fabbdddf) both tests pass, including test107's FWHM physics assertion. No source change is warranted. Why this was easy to misread: create_output_ref() calls mkdir(..., exist_ok=True), so a missing dataset leaves an empty directory rather than an error, and git does not track empty directories, so the submodule looked clean. - found-bugs: add B-009 (environment cause, resolved) and B-008 (the reporting gap, open), and extend the false-positive table with the 'submodule behind the recorded pointer' check. - environment-setup: new section 2.1 to compare the submodule HEAD with 'git ls-tree HEAD opengate/tests/data', plus a checklist item. - running-tests: rule out environment causes (submodule/Geant4/stale .so) before believing a failure. - project-status: the 366/368 baseline is an environment artifact; record the green re-run. - task-list: T-014 and T-020 done, T-021 opened for the reporting gap.
BishopWolf
marked this pull request as draft
September 22, 2026 11:17
Document the skills directory and add role-oriented playbooks that answer 'how should I think about this?' on top of the existing task-oriented skills. - skills/README.md: full layout of skills/, a 'which skill do I need?' table, the task-vs-role split, the conventions every skill follows (machine-agnostic paths, classify before diagnosing, evidence over claims), and how to add a skill. - skills/engineering/python-gate-developer.md: where each kind of change belongs, the hard design rules (no G4 objects in the user phase, __initcpp__ vs __init__, Python base class first, dict registries, lazy heavy imports), adding a user-facing parameter, working across the C++ boundary, definition of done. - skills/engineering/geant4-physics-expert.md: prove the environment first, where physics lives in the tree, how to justify a physics-list choice, cuts and thresholds, statistics and uncertainty as the most common physics mistake, physics-sensitive checks. - skills/engineering/simulation-debugger.md: triage (environment vs repository bug) as step one, minimal deterministic reproduction, locating the failure in the simulation lifecycle, getting more information, the ranked list of real causes, how to report. - skills/engineering/test-writer.md: placement and numbering, proving the test can fail, what to assert on, justified tolerances, a template, the suite's constraints, and verification under multiprocessing. All four cite real APIs (user_info setters, random_seed/current_random_seed, g4_verbose_level, to_json_file, opengate/tests/utility.py) verified against the tree, and reference the known defects (B-001..B-009) where relevant. Also register the new files in the AGENTS.md skills index and in the project-status asset table, and add T-022 to the task list. All relative links across skills/ and AGENTS.md were checked and resolve.
…-setup
Skills changes only (no opengate/ or core/ code touched).
New role skills under skills/engineering/, each grounded in real class
and parameter names read from the code (SolidBase/BoxSolid..., GenericSource,
DoseActor, reference_physics_list_*):
- gate-physics-expert.md GATE physics layer: physics manager, sources
(all source types, validators, normalisation),
actors (dose + digitizer families), filters, biasing
- geant4-geometry-expert.md raw G4 geometry: object model vs GATE, C++ side in
core/, placement/repetition, overlaps, navigation
- gate-geometry-expert.md GATE geometry layer: volume tree, solids, materials
(incl. HU tables), regions, fields, silent-failure checks
- SKILL.md index: which role to pick, the two layered pairs
(geant4-* vs gate-*), role composition, conventions
Merge build-and-ci into environment-setup:
- skills/build-and-ci/ removed; its content became environment-setup §6
(Build, packaging and CI: topology, release, what CI tests, reproduce locally,
core/ packaging consequences, safe workflow edits). The two skills duplicated
the CI-reproduction recipe; there is now one owner per fact.
- environment-setup header states the two-part scope.
- Corrected build-and-ci's `-r` description to match the real help text
("the last 10 tests and 1/4 of the others randomly").
Fix .gitignore (B-011): `build*/` also matched skills/build-and-ci/, silently
making any file in that directory un-committable - the hidden cause of B-010.
Added `!skills/build*-*/`; verified real build/dist/output paths stay ignored.
Docs/status:
- skills/README.md: layout, which-skill table, "easily-confused pairs" boundary table
- AGENTS.md §3: skills index updated, environment/build routing clarified
- python-gate-developer.md: links to architecture §3/§7 and code-style instead
of restating them
- status: T-023/T-024/T-026/T-027/T-028 recorded; B-010 superseded, B-011 closed;
B-002/B-005 diagnosed and proved at runtime but left unapplied per scope (T-029)
Evidence: link check over skills/ + AGENTS.md -> 0 broken; no absolute
machine paths in committed files (T-011); pre-commit clean.
Contributor
Author
|
I have added an example of use of these changes in the issue #1115 |
…drift
Add the missing gate-chemistry-expert role skill and enforce the
"SKILL.md is an index, deeper topics get their own file" convention on
environment-setup, which had grown to 862 lines.
- skills/engineering/gate-chemistry-expert.md (new): the GATE chemistry
layer -- Geant4-DNA chemistry lists, chemistry world/scavengers,
track-structure EM regions, chemistry actors and counters. Grounded in
real code names; carries the alpha-stage warning.
- skills/environment-setup/ split into an index plus two deep-dives:
* geant4-itk.md (new): build Geant4/ITK and rebuild opengate_core
* build-and-ci.md (new): packaging, releasing, CI topology
SKILL.md 862 -> 510 lines; pointer stubs at sections 4 and 6. Section
numbering re-verified contiguous 0-9.
- Repoint every reference to the moved sections (4.5, 4.6, 6.x) across
AGENTS.md, skills/README.md, the engineering role skills and the status
files; document the split convention in skills/README.md.
- status/found-bugs.md: remove the duplicated B-002 entry (merge its
verified-fix note into the primary entry), add the missing B-009 entry,
and move B-010/B-011 out of the Open table into Closed.
- status/project-status.md: drop the duplicated environment-setup row.
- Fixes introduced and corrected during the split: the cron trigger is
quoted from main.yml as '0 0 * 0,3', and the LD_PRELOAD TLS example
keeps its /lib path.
Link scan over skills/ + AGENTS.md: 0 broken. pre-commit clean.
…traps Verification round on the OpenGATE issue tracker (112 open issues), plus the lessons learned while doing it. All verdicts cite what was actually run or read. found-bugs.md - New "Upstream issue triage" section: a per-issue verdict table, U-001..U-010 entries, and a fourth verdict vocabulary (NOT REPRODUCED / NOT VERIFIED / CONFIRMED / fixed-on-master). - Verified with real runs on 996572b (GATE 10.1.1, Geant4 11.4.2): * OpenGATE#1107 Tubs vs area-matched Hexagon -> NOT REPRODUCED (ratio 1.0000, 3 seeds) * OpenGATE#857 RepeatParametrisedVolume deletes ROOT -> NOT REPRODUCED (repeater is innocent; the missing file came from a zero-hit source) * OpenGATE#1035 DigitizerHitsCollectionActor ROOT not finalized -> NOT REPRODUCED (End == size, uproot reads Hits;1) * OpenGATE#1115 ShieldingLIQMD_HP_EMZ -> NOT REPRODUCED on the pinned Geant4 * OpenGATE#1059 >2^31 primaries -> already fixed (GetPlatformMaxPrimariesPerRun + warning + source/test098 -> 1/1) * OpenGATE#616 scale_itk_image aliasing -> already fixed (array_from_image = copy) - OpenGATE#1135 TLEDoseActor thread race: real, fixed on master by a92a62b (thread_local caches at GateMaterialMuHandler.cpp:69-71) but contained in NO release tag -- a tree ahead of the release would look clean and wrongly dismiss it. - OpenGATE#1143 mutex contention on ROOT write-out: CONFIRMED, promoted to the active bug table (unlocked singleton, SetNtupleMerging + 4000-entry basket, AddNtupleRow per row). skills/engineering - simulation-debugger.md 5.1: harness-vs-repository-bug table; validate a reproduction against a known answer first. - simulation-debugger.md 5.2: read the issue comments before reproducing; a claim about a released version cannot be tested on a tree ahead of it. - gate-physics-expert.md 4.1/4.2: beam/source harness traps (beam2d is not a parallel beam; number_of_primaries vs activity; Run0254 when instantiating a physics list outside a simulation). - gate-geometry-expert.md 4.1/4.2: boolean operands must not be placed; Tubs/Hexagon area formulas and the circumradius trap. - test-writer.md 7: the same validation rules for scratch reproductions. task-list.md: T-032 (triage) and T-033 (record the traps) marked done. skills/ only; no product code touched.
Contributor
Author
Collaborator
|
Thanks a bunch @BishopWolf for picking this up. This is really important. I suggest that @dsarrut, @tbaudier, and I do a pass over the agent instruction files you already created and review and enhance them. From my experience with local agents (Codex in VS Code in my case), they pick up the overall architecture well and also the simulation workflow, but not all the subtleties - although this is getting better with each model generation. |
The extras were being installed piecemeal, so a partial install looked like
success: the torch-dependent tests were the visible gap, and the missing
pytomography/hist/pyvista/pymedphys packages were reported as "still
uncovered" instead of "not installed", although CI installs all of them. The
skill now takes the list from CI, audits the whole test tree, and documents the
per-platform torch/numpy constraint.
skills/environment-setup/SKILL.md
- 5.0: the authoritative extras list is read from
.github/workflows/actions_tests/action.yml, not from memory.
- 5.1: an AST audit of every module imported under opengate/tests/src, each
checked with importlib.util.find_spec; a complete environment must print
NOTHING MISSING. Documents the three known local-module false positives
(actors, test043_garf_helpers, coresi) and the discovered-file count check
(368 -> 376 once the extras were in). A skipped test prints its reason
("--> Torch not avail"): read it, do not assume.
- 5.2: the torch/numpy ABI mismatch is per-platform, so no portable version
list exists. PyTorch publishes no Intel-macOS (x86_64) wheel newer than
2.2.2, which forces numpy<2 there, while Linux and Apple Silicon take a
current torch on NumPy 2. The procedure is resolver-driven: dry-run the whole
partner set, upgrade torch if a newer wheel exists, otherwise dry-run
"numpy<2 torch" together with its partners, then re-verify the bridge; never
copy the versions to another machine. Says why CI cannot catch it: torch is
installed only on ubuntu-24.04, windows-2025 and macos-15 (arm).
skills/status/found-bugs.md
- B-012 (environment, not a repository bug): torch 2.2.2 against numpy 2.4.4
made every NumPy<->torch bridge call raise "RuntimeError: Numpy is not
available" ("A module that was compiled using NumPy 1.x ..."), silently
skipping the 11 torch-dependent tests. Resolved with numpy 1.26.4 +
pandas 2.3.3 + scipy 1.12.0 + torch 2.2.2, recorded together with its
platform (macOS-15.8-x86_64) and marked machine-specific.
Evidence: bridge OK, the runner discovers 376 files (was 368), test081 1/1,
the 7-test GAN/garf batch 8/8, external/pytomography/test094d_reconstruction
1/1 (128.8 s), the 5.1 audit prints NOTHING MISSING.
- Added the matching symptom row to the "harness or environment?" triage table.
skills/status/task-list.md
- T-034 done (install and verify the extras, then run the dependent tests).
- T-009 unblocked and done: moved out of the now-empty Blocked table into Active.
- T-035 opened: 28 wip-flagged files are never executed by the runner
(get_ignored_patterns); finish-and-rename or delete.
- Corrected that entry's inventory while committing: 28 wip files is 27 tests
plus the helper actors/test043_garf_helpers_wip.py (not "28 + 3 helpers"),
and the test053/test056/test060 files live in source/, not physics/;
verified with find opengate/tests/src -name '*wip*.py'. The note that
get_torch_dependent_tests() lists three files absent from the tree was
confirmed too.
skills/ only; no product code touched. pre-commit clean (trailing-whitespace).
core/external/fmt is left dirty (untracked support/.gradle cache inside the
submodule) and is deliberately not staged.
Contributor
Author
|
I added instruction for agents to properly install torch and other requirements to consider the development environment complete |
Contributor
Author
|
Instructions on the use of MacPorts on MacOS Intel following the drop of homebrew support. For Apple Silicon nothing changes |
Contributor
It seems removing homebrew to brew is sufficient: |
Contributor
Author
Nice catch! I was wondering why everything stopped working altogether. I'll review it later, MacPorts works fine BTW |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Following discussion on agent skills in #951
This is a proposal for agent skills, it misses the agent scope, as it is to be discussed. Let me know if you don't agree on something.
I propose the scope is limited to test creation/validation, documentation, bug finding, critical bug correction, and development helpers
Please don't use agents to develop a new functionality.
I have verified than an agent can create on its own a virtual environment; run the tests and report status, bugs and task lists
NOTE: The status folder will be removed before merging, as it is a per user task list. its only here temporarily to prove the agents work
Issues like #1115 are the typical target for agents to research
Closes #951