From 81eb14d75c4461a078f9f25975ff1d454ccfa138 Mon Sep 17 00:00:00 2001 From: Lars Bogner Date: Thu, 13 Aug 2026 10:31:47 +0200 Subject: [PATCH] Move scripts/ to giant/tools/ (issues.md Issue 9) `scripts` was published as a top-level distribution package, colliding with one of the most generic names in the Python ecosystem and shadowable by a stray scripts/ dir on the portal machines' shared /work/lbogner. Move it under the giant namespace; the dwarf command name is unchanged, only the Python import path and file location move. Co-Authored-By: Claude Sonnet 5 --- CLAUDE.md | 2 +- README.md | 2 +- giant/analysis/runtime_estimate.py | 2 +- giant/cli.py | 2 +- giant/config.py | 2 +- giant/data/loader.py | 2 +- {scripts => giant/tools}/__init__.py | 0 .../tools}/bump_dataset_version.py | 4 +-- .../tools}/check_migration_v02_v03.py | 6 ++-- {scripts => giant/tools}/create_root_files.py | 2 +- {scripts => giant/tools}/dwarf.py | 18 ++++++------ {scripts => giant/tools}/geometry_oracle.py | 0 {scripts => giant/tools}/hparam_scan.py | 0 .../tools}/migrate_geant_steps.py | 0 .../tools}/profile_analysis_costs.py | 2 +- {scripts => giant/tools}/steps_to_parquet.py | 0 .../tools}/steps_to_parquet_parallel.py | 10 +++---- {scripts => giant/tools}/warm_setup_cache.py | 0 issues.md | 29 ++++++++++++++++++- pyproject.toml | 6 ++-- tests/test_bump_dataset_version.py | 2 +- tests/test_create_root_files.py | 2 +- tests/test_dataset.py | 2 +- tests/test_dwarf.py | 6 ++-- tests/test_steps_to_parquet.py | 2 +- tests/test_steps_to_parquet_parallel.py | 2 +- 26 files changed, 66 insertions(+), 39 deletions(-) rename {scripts => giant/tools}/__init__.py (100%) rename {scripts => giant/tools}/bump_dataset_version.py (99%) rename {scripts => giant/tools}/check_migration_v02_v03.py (96%) rename {scripts => giant/tools}/create_root_files.py (99%) rename {scripts => giant/tools}/dwarf.py (96%) rename {scripts => giant/tools}/geometry_oracle.py (100%) rename {scripts => giant/tools}/hparam_scan.py (100%) rename {scripts => giant/tools}/migrate_geant_steps.py (100%) rename {scripts => giant/tools}/profile_analysis_costs.py (99%) rename {scripts => giant/tools}/steps_to_parquet.py (100%) rename {scripts => giant/tools}/steps_to_parquet_parallel.py (95%) rename {scripts => giant/tools}/warm_setup_cache.py (100%) diff --git a/CLAUDE.md b/CLAUDE.md index 8b73e3c..33106f1 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -22,7 +22,7 @@ giant analyze render --gallery # render PDFs + HTML dwarf --help # dataset/tooling CLI: convert, migrate, bump-gen, # bump-schema, status, update-manifest, create-manifest, # make-root, build-geometry-oracle, warm-cache, hparam-scan - # (see scripts/dwarf.py) + # (see giant/tools/dwarf.py) ``` `cpu` and `cuda` are mutually exclusive — pick one to select the torch build (pinned to 2.3.x; newer torch requires newer NVIDIA drivers). Plain `uv sync` with no extra will not install torch at all; uv has no concept of a "default extra", so `--extra cpu` should always be included unless you need GPU support. diff --git a/README.md b/README.md index c2244f5..cf018db 100644 --- a/README.md +++ b/README.md @@ -93,7 +93,7 @@ giant/ │ │ ├── condor.py # prep / compute-one / submit-description plumbing │ │ └── render.py # PDFs + HTML gallery (only module importing plotstyle/LaTeX) │ └── cli.py # `giant train` / `new-run` / `predict` / `rollout` / `analyze` Typer app -├── scripts/ # dataset/tooling logic, unified under the `dwarf` CLI (`dwarf --help`) +├── giant/tools/ # dataset/tooling logic, unified under the `dwarf` CLI (`dwarf --help`) │ ├── dwarf.py # Typer app: convert, migrate, bump-gen, bump-schema, status, │ │ # update-manifest, create-manifest, make-root, │ │ # build-geometry-oracle, warm-cache, hparam-scan diff --git a/giant/analysis/runtime_estimate.py b/giant/analysis/runtime_estimate.py index 7f6cc37..ae703be 100644 --- a/giant/analysis/runtime_estimate.py +++ b/giant/analysis/runtime_estimate.py @@ -27,7 +27,7 @@ would then wrongly scale up with a bigger dataset. `RUNTIME_SAFETY_MARGIN` is deliberately generous (4x total) specifically to absorb that kind of contention spike instead. Rerun this calibration (pull fresh `condor_history`/`run_meta.json`, refit) if the catalog changes or timings -drift — a synthetic local rebaseline via `scripts/profile_analysis_costs.py` +drift — a synthetic local rebaseline via `giant/tools/profile_analysis_costs.py` is a reasonable fallback when no real cluster data is available yet, but undershoots real wall time badly (it can't see docker pull / `/ceph` I/O latency), which is exactly why this file moved off it. diff --git a/giant/cli.py b/giant/cli.py index 6b6bab2..764399b 100644 --- a/giant/cli.py +++ b/giant/cli.py @@ -228,7 +228,7 @@ class Stage1Context(str, Enum): # Conditioning itself lives in giant.config (imported below as gconfig) — -# shared with scripts/dwarf.py's Typer commands so the two CLIs can't +# shared with giant/tools/dwarf.py's Typer commands so the two CLIs can't # silently drift apart on the option's valid values. Conditioning = gconfig.Conditioning diff --git a/giant/config.py b/giant/config.py index 0f0ae77..fecfd72 100644 --- a/giant/config.py +++ b/giant/config.py @@ -19,7 +19,7 @@ from giant._migration import V02_FIXED_FACTS, V02_MODEL_KEY_TO_STAGES, reject_le class Conditioning(str, Enum): """`conditioning.particle.type` / `conditioning.material.type` choices — - shared by `giant.cli` and `scripts.dwarf`'s Typer commands so the two + shared by `giant.cli` and `giant.tools.dwarf`'s Typer commands so the two CLIs can't silently drift apart on the option's valid values (see DEFAULT_CONFIG["conditioning"] for what each value means).""" diff --git a/giant/data/loader.py b/giant/data/loader.py index 94acfba..094ccd5 100644 --- a/giant/data/loader.py +++ b/giant/data/loader.py @@ -16,7 +16,7 @@ from giant.constants import K_MAX MANIFEST_SUFFIX = ".manifest" # Each input parquet file is a separate Geant4 job converted 1:1 from its own -# ROOT file (scripts/steps_to_parquet.py), and a job's event_id numbering +# ROOT file (giant/tools/steps_to_parquet.py), and a job's event_id numbering # always restarts from 0 — so when multiple files are loaded together (a # directory or .manifest), raw event_id values collide across files even # though they refer to unrelated events. Every per-file event_id column gets diff --git a/scripts/__init__.py b/giant/tools/__init__.py similarity index 100% rename from scripts/__init__.py rename to giant/tools/__init__.py diff --git a/scripts/bump_dataset_version.py b/giant/tools/bump_dataset_version.py similarity index 99% rename from scripts/bump_dataset_version.py rename to giant/tools/bump_dataset_version.py index 27ecabe..1e362c9 100644 --- a/scripts/bump_dataset_version.py +++ b/giant/tools/bump_dataset_version.py @@ -1,5 +1,5 @@ """Cut a new raw generation or processed schema version for the geant_steps -dataset tree (see scripts/migrate_geant_steps.py for the layout): +dataset tree (see giant/tools/migrate_geant_steps.py for the layout): raw////shard-NNN.root processed/////shard-NNN.parquet @@ -579,7 +579,7 @@ def apply_create_manifest(output_path: Path, lines: list[str]) -> None: # --------------------------------------------------------------------------- -# CLI entry points (called from scripts/dwarf.py) +# CLI entry points (called from giant/tools/dwarf.py) # --------------------------------------------------------------------------- diff --git a/scripts/check_migration_v02_v03.py b/giant/tools/check_migration_v02_v03.py similarity index 96% rename from scripts/check_migration_v02_v03.py rename to giant/tools/check_migration_v02_v03.py index 3386a91..dd36f8a 100644 --- a/scripts/check_migration_v02_v03.py +++ b/giant/tools/check_migration_v02_v03.py @@ -10,9 +10,9 @@ machine against an actual trained checkpoint before merging Usage (from the repo root, on a portal machine): - uv run python scripts/check_migration_v02_v03.py /ceph/lbogner/.../best.pt - uv run python scripts/check_migration_v02_v03.py /ceph/lbogner/.../best.pt --ema - uv run python scripts/check_migration_v02_v03.py /ceph/lbogner/.../best.pt --batch 32 --seed 1 + uv run python giant/tools/check_migration_v02_v03.py /ceph/lbogner/.../best.pt + uv run python giant/tools/check_migration_v02_v03.py /ceph/lbogner/.../best.pt --ema + uv run python giant/tools/check_migration_v02_v03.py /ceph/lbogner/.../best.pt --batch 32 --seed 1 Run it once against a flow (or ddpm) checkpoint and once against a wgan checkpoint ("one flow checkpoint and one WGAN checkpoint"). diff --git a/scripts/create_root_files.py b/giant/tools/create_root_files.py similarity index 99% rename from scripts/create_root_files.py rename to giant/tools/create_root_files.py index 66ba11a..4e54a2a 100644 --- a/scripts/create_root_files.py +++ b/giant/tools/create_root_files.py @@ -32,7 +32,7 @@ from dataclasses import dataclass from concurrent.futures import ThreadPoolExecutor, as_completed from pathlib import Path -# Must match scripts/bump_dataset_version.py's GEN_RE. +# Must match giant/tools/bump_dataset_version.py's GEN_RE. GEN_RE = re.compile(r"^gen\d+$") SHARD_RE = re.compile(r"^shard-(\d+)\.root$") diff --git a/scripts/dwarf.py b/giant/tools/dwarf.py similarity index 96% rename from scripts/dwarf.py rename to giant/tools/dwarf.py index 674e252..5983735 100644 --- a/scripts/dwarf.py +++ b/giant/tools/dwarf.py @@ -1,6 +1,6 @@ """dwarf — little helper to `giant`: dataset/tooling CLI for the geant_steps pipeline. -Unifies the standalone scripts/*.py conversion, migration, versioning, and +Unifies the standalone giant/tools/*.py conversion, migration, versioning, and simulation-fanout tools into one Typer app so there's a single command name (and `--help`) to remember instead of five differently-hyphenated ones. """ @@ -14,20 +14,20 @@ import typer from typing_extensions import Annotated from giant.config import Conditioning -from scripts.bump_dataset_version import ( +from giant.tools.bump_dataset_version import ( run_bump_gen, run_bump_schema, run_create_manifest, run_status, run_update_manifest, ) -from scripts.create_root_files import run_make_root -from scripts.geometry_oracle import run_build_geometry_oracle -from scripts.hparam_scan import DATA_DEFAULT, SCAN_DIR_DEFAULT, run_hparam_scan -from scripts.migrate_geant_steps import run_migration -from scripts.steps_to_parquet import convert_steps_to_parquet -from scripts.steps_to_parquet_parallel import run_parallel_job -from scripts.warm_setup_cache import run_warm_setup_cache +from giant.tools.create_root_files import run_make_root +from giant.tools.geometry_oracle import run_build_geometry_oracle +from giant.tools.hparam_scan import DATA_DEFAULT, SCAN_DIR_DEFAULT, run_hparam_scan +from giant.tools.migrate_geant_steps import run_migration +from giant.tools.steps_to_parquet import convert_steps_to_parquet +from giant.tools.steps_to_parquet_parallel import run_parallel_job +from giant.tools.warm_setup_cache import run_warm_setup_cache app = typer.Typer(no_args_is_help=True) diff --git a/scripts/geometry_oracle.py b/giant/tools/geometry_oracle.py similarity index 100% rename from scripts/geometry_oracle.py rename to giant/tools/geometry_oracle.py diff --git a/scripts/hparam_scan.py b/giant/tools/hparam_scan.py similarity index 100% rename from scripts/hparam_scan.py rename to giant/tools/hparam_scan.py diff --git a/scripts/migrate_geant_steps.py b/giant/tools/migrate_geant_steps.py similarity index 100% rename from scripts/migrate_geant_steps.py rename to giant/tools/migrate_geant_steps.py diff --git a/scripts/profile_analysis_costs.py b/giant/tools/profile_analysis_costs.py similarity index 99% rename from scripts/profile_analysis_costs.py rename to giant/tools/profile_analysis_costs.py index 60dda89..9a37f1b 100644 --- a/scripts/profile_analysis_costs.py +++ b/giant/tools/profile_analysis_costs.py @@ -13,7 +13,7 @@ real checkpoint) they short-circuit almost instantly and are excluded here — see `runtime_estimate.py`'s `_ROUTER_FIXED_S` for how those are handled instead. -Usage: ``uv run python scripts/profile_analysis_costs.py`` +Usage: ``uv run python giant/tools/profile_analysis_costs.py`` """ from __future__ import annotations diff --git a/scripts/steps_to_parquet.py b/giant/tools/steps_to_parquet.py similarity index 100% rename from scripts/steps_to_parquet.py rename to giant/tools/steps_to_parquet.py diff --git a/scripts/steps_to_parquet_parallel.py b/giant/tools/steps_to_parquet_parallel.py similarity index 95% rename from scripts/steps_to_parquet_parallel.py rename to giant/tools/steps_to_parquet_parallel.py index 1817c36..10794cd 100644 --- a/scripts/steps_to_parquet_parallel.py +++ b/giant/tools/steps_to_parquet_parallel.py @@ -2,11 +2,11 @@ A single `dwarf convert` call converts a list of files one at a time; this module runs up to --jobs conversions concurrently, each as its own `dwarf -convert` subprocess (invoked via `python -m scripts.dwarf`, so it picks up +convert` subprocess (invoked via `python -m giant.tools.dwarf`, so it picks up the active venv/uv environment automatically). Inputs must live under /raw////.root -(see scripts/migrate_geant_steps.py) — each is written to the matching +(see giant/tools/migrate_geant_steps.py) — each is written to the matching processed/////.parquet, where defaults to the highest schemaN already under processed/// (pass --schema to pick a specific one, e.g. one just created by `dwarf bump-schema`). @@ -22,7 +22,7 @@ import sys from concurrent.futures import ThreadPoolExecutor, as_completed from pathlib import Path -# Must match scripts/bump_dataset_version.py's GEN_RE / SCHEMA_RE. +# Must match giant/tools/bump_dataset_version.py's GEN_RE / SCHEMA_RE. GEN_RE = re.compile(r"^gen\d+$") SCHEMA_RE = re.compile(r"^schema(\d+)$") @@ -82,7 +82,7 @@ def resolve_destination(root_file: Path, dataset_root: Path, schema_override: st return processed_gen_dir / schema_tag / detector / f"{shard_stem}.parquet" -_DWARF_CONVERT_CMD = [sys.executable, "-m", "scripts.dwarf", "convert"] +_DWARF_CONVERT_CMD = [sys.executable, "-m", "giant.tools.dwarf", "convert"] def _convert_one( @@ -127,7 +127,7 @@ def run_parallel( written next to the input .root). *cmd_prefix* overrides the subprocess command run per file (defaults to - `python -m scripts.dwarf convert`) — used by tests to substitute a fake + `python -m giant.tools.dwarf convert`) — used by tests to substitute a fake conversion script. Returns one (root_file, returncode, stdout, stderr) tuple per file, in diff --git a/scripts/warm_setup_cache.py b/giant/tools/warm_setup_cache.py similarity index 100% rename from scripts/warm_setup_cache.py rename to giant/tools/warm_setup_cache.py diff --git a/issues.md b/issues.md index b4611db..3c78d2f 100644 --- a/issues.md +++ b/issues.md @@ -48,7 +48,7 @@ architecture matrix grows, not about rot or breakage. | 6 | Two independent v0.2→v0.3 migration surfaces encode the same knowledge | Medium | Medium | **Fixed** | | 7 | Positional tuple contracts (9-tuple, 7-tuple) between data, model and training layers | Medium | Small | **Fixed** | | 8 | `network.py` is 1745 lines holding three distinct modules | Medium | Small | **Fixed** | -| 9 | `scripts` is published as a top-level distribution package | Medium | Small | Open | +| 9 | `scripts` is published as a top-level distribution package | Medium | Small | **Fixed** | | 10 | `torch.load(weights_only=False)` — checkpoints are arbitrary pickles | Low | Medium | Open | | 11 | Minor: `echo=print` threading, `particles → data.loader` layering | Low | Small | Open | @@ -1131,6 +1131,33 @@ changes is effectively unreviewable. ## Issue 9 — `scripts` is published as a top-level distribution package +> **Status: Fixed.** `scripts/` moved to `giant/tools/` via `git mv` (preserving file +> history); `pyproject.toml`'s `[project.scripts]` entry point is now +> `dwarf = "giant.tools.dwarf:app"`, `[tool.hatch.build.targets.wheel] packages` is now +> `["giant"]` only, and `[tool.coverage.run] source` is now `["giant"]` only. The 7 +> `from scripts.X import Y` intra-package imports in `dwarf.py` became +> `from giant.tools.X import Y`; the one live (non-cosmetic) reference to the old module +> path — `steps_to_parquet_parallel.py`'s `_DWARF_CONVERT_CMD` subprocess invocation +> (`python -m scripts.dwarf convert`, used by every `dwarf convert --jobs N` fan-out +> worker) — is now `python -m giant.tools.dwarf`. The 5 test files that imported from +> `scripts` (`test_dwarf.py`, `test_steps_to_parquet.py`, +> `test_steps_to_parquet_parallel.py`, `test_bump_dataset_version.py`, +> `test_create_root_files.py`) now import from `giant.tools`. All remaining prose +> references to `scripts/`/`scripts.` — in `giant/cli.py`, `giant/config.py`, +> `giant/data/loader.py`, `giant/analysis/runtime_estimate.py`, `tests/test_dataset.py`, +> `CLAUDE.md`, `README.md`'s repo-tree block, and docstrings/comments inside the moved +> files themselves (`dwarf.py`, `profile_analysis_costs.py`, `check_migration_v02_v03.py`, +> `bump_dataset_version.py`, `create_root_files.py`) — were updated to `giant/tools/` / +> `giant.tools.` for accuracy. `giant/analysis/condor.py` and +> `.gitea/workflows/ci.yml` were checked per the scope guard below and needed no changes +> (neither hard-codes a `scripts/` path; CI's coverage source comes from `pyproject.toml`). +> The `dwarf` **command name is unchanged** — `giant train --help`/`dwarf --help` behavior +> is identical, only the Python import path and file location moved. `uv sync --extra cpu +> --extra dev` reinstalls cleanly with the new entry point; `uv run pytest -q` (804 +> passed, unchanged), `uv run ruff check .`, `uv run ruff format --check .`, and `uv run ty +> check .` all clean. Everything below this point describes the pre-fix state and is kept +> for historical context. + **Severity: Medium. Effort: Small.** **Location:** `pyproject.toml`: diff --git a/pyproject.toml b/pyproject.toml index 1d7538b..c9cd7ce 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -50,13 +50,13 @@ analysis = [ [project.scripts] giant = "giant.cli:app" -dwarf = "scripts.dwarf:app" +dwarf = "giant.tools.dwarf:app" [tool.ruff] line-length = 120 [tool.coverage.run] -source = ["giant", "scripts"] +source = ["giant"] omit = ["*/legacy/*"] [tool.coverage.report] @@ -70,7 +70,7 @@ requires = ["hatchling"] build-backend = "hatchling.build" [tool.hatch.build.targets.wheel] -packages = ["giant", "scripts"] +packages = ["giant"] [tool.uv] conflicts = [ diff --git a/tests/test_bump_dataset_version.py b/tests/test_bump_dataset_version.py index 83f7dd9..6b48bf7 100644 --- a/tests/test_bump_dataset_version.py +++ b/tests/test_bump_dataset_version.py @@ -1,7 +1,7 @@ import os import subprocess -from scripts import bump_dataset_version +from giant.tools import bump_dataset_version plan_bump_gen = bump_dataset_version.plan_bump_gen plan_bump_schema = bump_dataset_version.plan_bump_schema diff --git a/tests/test_create_root_files.py b/tests/test_create_root_files.py index 686e198..6b02075 100644 --- a/tests/test_create_root_files.py +++ b/tests/test_create_root_files.py @@ -4,7 +4,7 @@ from pathlib import Path import pytest -from scripts import create_root_files +from giant.tools import create_root_files parse_detector_spec = create_root_files.parse_detector_spec next_shard_index = create_root_files.next_shard_index diff --git a/tests/test_dataset.py b/tests/test_dataset.py index 5d2e71e..02f3666 100644 --- a/tests/test_dataset.py +++ b/tests/test_dataset.py @@ -95,7 +95,7 @@ def _dummy_normalizer(width): def test_streaming_dataset_offsets_colliding_event_ids_across_files(tmp_path): """Two files that each restart event_id from 0 (one Geant4 job per file, - see scripts/steps_to_parquet.py) must not have their same-numbered events + see giant/tools/steps_to_parquet.py) must not have their same-numbered events collapsed together: every row from every file must show up in exactly one of train/val, and the number of distinct events must be the sum across files, not the union of raw ids.""" diff --git a/tests/test_dwarf.py b/tests/test_dwarf.py index 2d9aeae..e312e0f 100644 --- a/tests/test_dwarf.py +++ b/tests/test_dwarf.py @@ -3,15 +3,15 @@ from typer.testing import CliRunner from giant import cli as giant_cli from giant.config import Conditioning from giant.data import setup_cache -from scripts import dwarf -from scripts.dwarf import app +from giant.tools import dwarf +from giant.tools.dwarf import app from test_pipeline import _make_synthetic_steps runner = CliRunner() def test_conditioning_enum_shared_across_both_clis(): - """giant.cli and scripts.dwarf must use the one giant.config.Conditioning + """giant.cli and giant.tools.dwarf must use the one giant.config.Conditioning enum, not independently redefined copies that could silently drift apart on valid --conditioning values.""" assert dwarf.Conditioning is Conditioning diff --git a/tests/test_steps_to_parquet.py b/tests/test_steps_to_parquet.py index 169bda8..19d7921 100644 --- a/tests/test_steps_to_parquet.py +++ b/tests/test_steps_to_parquet.py @@ -1,6 +1,6 @@ import polars as pl -from scripts import steps_to_parquet +from giant.tools import steps_to_parquet def _frame() -> pl.DataFrame: diff --git a/tests/test_steps_to_parquet_parallel.py b/tests/test_steps_to_parquet_parallel.py index 101d36f..7a7a857 100644 --- a/tests/test_steps_to_parquet_parallel.py +++ b/tests/test_steps_to_parquet_parallel.py @@ -2,7 +2,7 @@ import json import sys from pathlib import Path -from scripts import steps_to_parquet_parallel +from giant.tools import steps_to_parquet_parallel run_parallel = steps_to_parquet_parallel.run_parallel resolve_destination = steps_to_parquet_parallel.resolve_destination