Move scripts/ to giant/tools/ (issues.md Issue 9)
CI / Format (ruff format) (push) Successful in 27s
CI / Lint (ruff check) (push) Successful in 28s
CI / Sync project version with tag (push) Has been skipped
CI / Lint (ruff check) (pull_request) Successful in 34s
CI / Type check (ty) (push) Successful in 38s
CI / Format (ruff format) (pull_request) Successful in 52s
CI / Sync project version with tag (pull_request) Has been skipped
CI / Type check (ty) (pull_request) Successful in 51s
CI / Tests (pull_request) Successful in 3m26s
CI / Tests (push) Successful in 3m36s
CI / Format (ruff format) (push) Successful in 27s
CI / Lint (ruff check) (push) Successful in 28s
CI / Sync project version with tag (push) Has been skipped
CI / Lint (ruff check) (pull_request) Successful in 34s
CI / Type check (ty) (push) Successful in 38s
CI / Format (ruff format) (pull_request) Successful in 52s
CI / Sync project version with tag (pull_request) Has been skipped
CI / Type check (ty) (pull_request) Successful in 51s
CI / Tests (pull_request) Successful in 3m26s
CI / Tests (push) Successful in 3m36s
`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 <noreply@anthropic.com>
This commit is contained in:
@@ -22,7 +22,7 @@ giant analyze render <run_dir> --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.
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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.
|
||||
|
||||
+1
-1
@@ -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
|
||||
|
||||
|
||||
+1
-1
@@ -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)."""
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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/<kind>/<gen>/<detector>/shard-NNN.root
|
||||
processed/<kind>/<gen>/<schema>/<detector>/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)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@@ -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").
|
||||
@@ -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$")
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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
|
||||
@@ -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 <dataset-root>/raw/<kind>/<gen>/<detector>/<file>.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/<kind>/<gen>/<schema>/<detector>/<file>.parquet, where <schema>
|
||||
defaults to the highest schemaN already under processed/<kind>/<gen>/ (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
|
||||
@@ -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`:
|
||||
|
||||
+3
-3
@@ -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 = [
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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."""
|
||||
|
||||
+3
-3
@@ -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
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user