Skip to content

Commit e604463

Browse files
committed
Implement mode-agnostic refactor
1 parent d90162a commit e604463

File tree

9 files changed

+204
-92
lines changed

9 files changed

+204
-92
lines changed

.gitignore

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
uv.lock
44
/data/*/**
55
/out/*/*/**
6-
/tmp/
6+
/old/*
77

88
# Ignore Python cache files
99
__pycache__

README.md

Lines changed: 12 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,9 @@
22

33
## Configuration & Trials
44

5-
Configuration lives under `cfg/<mode>/...` where `<mode>` is one of:
6-
7-
- `sweep` – hyperparameter exploration (multiple non-seed dimensions allowed; seed must be scalar)
8-
- `experiment` – focused evaluation (only the seed may vary; other hyperparameters must be scalar)
5+
Configuration lives under `cfg/<mode>/...`.
6+
The `<mode>` segment is only a folder namespace (e.g., `sweep`, `experiment`, `sweep_1`).
7+
It does not affect expansion semantics.
98

109
Directory pattern for a leaf config:
1110

@@ -21,33 +20,27 @@ Expansion writes resolved per‑trial configurations to:
2120
out/log/<mode>/<dataset>/<model>/<trainer>/trial_###/cfg.yaml
2221
```
2322

24-
Trials are the Cartesian product of list‑valued hyperparameters excluding those on a structural allowlist (currently only `batch_metrics`). Special handling:
25-
26-
- `seed:` list -> becomes a sweep over seeds (each trial gets one scalar value)
27-
- `seed:` scalar -> required if no list sweep is desired
28-
29-
Mode rules enforced during expansion:
30-
31-
1. sweep mode: `seed` must be scalar; any other list hyperparameters are allowed.
32-
2. experiment mode: only `seed` may be list-valued; all other hyperparameters must be scalar (aside from structural allowlist like `batch_metrics`).
23+
Trials are the Cartesian product of list‑valued hyperparameters, excluding logging lists
24+
(currently only `batch_metrics`). `seed` may be scalar or list; if a list, it is just another
25+
dimension.
3326

3427
Re‑running expansion is idempotent: resolved trial `cfg.yaml` files are regenerated (so parent edits propagate). A trial is considered complete when `batch_log.csv` exists in the same `trial_###` directory.
3528

3629
Summary:
3730

3831
1. Hierarchical inheritance (leaf overrides parents).
39-
2. Lists -> sweep axes (except structural allowlist).
40-
3. A list-form `seed` defines a seed axis; else scalar `seed` required.
41-
4. Mode-specific constraints (see above) validated at expansion time.
42-
5. Resolved configs live only under `out/log/...` and drive all training & analysis.
32+
2. Lists -> sweep axes (except logging lists like `batch_metrics`).
33+
3. `seed` can be scalar or a list (a list becomes an axis like any other).
34+
4. `mode` is only a folder namespace and does not affect expansion behavior.
35+
5. Resolved configs live under `out/log/...` and drive all training & analysis.
4336

4437
Programmatic expansion + execution:
4538

4639
```python
4740
from src.run import run_trials
48-
# All modes (sweep + experiment) across all datasets
41+
# All configs across all datasets
4942
run_trials()
50-
# Only sweep mode for mnist
43+
# Only configs under cfg/sweep for mnist
5144
run_trials(datasets=["mnist"], modes=["sweep"])
5245
```
5346

run/experiment.py

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,21 @@
11
import sys
22
from collections.abc import Sequence
33

4-
from src.run import run_trials
4+
from src.run import discover_modes, run_trials
55

66

7-
def main(datasets: Sequence[str] | None = None) -> None: # pragma: no cover - thin wrapper
8-
"""Run all pending experiment trials for given datasets (or all)."""
9-
run_trials(datasets=datasets, modes=['experiment'])
7+
def main(datasets: Sequence[str] | None = None, *, dry_run: bool = False) -> None: # pragma: no cover - thin wrapper
8+
"""Run all pending trials under cfg modes containing 'experiment'."""
9+
exp_like = discover_modes('experiment')
10+
run_trials(datasets=datasets, modes=exp_like, dry_run=dry_run)
1011

1112

1213
if __name__ == '__main__': # pragma: no cover
13-
ds = sys.argv[1:] if len(sys.argv) > 1 else None
14-
main(ds)
14+
# usage: uv run -m run.experiment [dataset ...] [--dry-run]
15+
args = sys.argv[1:]
16+
dry = False
17+
if '--dry-run' in args:
18+
dry = True
19+
args = [a for a in args if a != '--dry-run']
20+
ds = args if args else None
21+
main(ds, dry_run=dry)

run/sweep.py

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,19 @@
11
import sys
22
from collections.abc import Sequence
33

4-
from src.run import run_trials
4+
from src.run import discover_modes, run_trials
55

66

77
def main(datasets: Sequence[str] | None = None, *, dry_run: bool = False) -> None: # pragma: no cover - thin wrapper
8-
"""Run all pending sweep trials for given datasets (or all)."""
9-
run_trials(datasets=datasets, modes=['sweep'], dry_run=dry_run)
8+
"""Run all pending trials under cfg modes containing 'sweep'."""
9+
# Discover modes with 'sweep' in their name (mode-agnostic semantics)
10+
sweep_like = discover_modes('sweep')
11+
run_trials(datasets=datasets, modes=sweep_like, dry_run=dry_run)
1012

1113

1214
if __name__ == '__main__': # pragma: no cover
1315
# usage: uv run -m run.sweep [dataset ...] [--dry-run]
16+
# 'sweep' is a folder namespace only; expansion is mode-agnostic.
1417
args = sys.argv[1:]
1518
dry = False
1619
if '--dry-run' in args:

src/run.py

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,11 @@
11
"""Training orchestration for uniform config expansion.
22
3-
Each leaf `cfg.yaml` under `cfg/<mode>/` is expanded (respecting mode rules) and
4-
written to `out/log/<mode>/<dataset>/<model>/<trainer>/trial_###/cfg.yaml`.
3+
Each leaf `cfg.yaml` under `cfg/<mode>/` is expanded and written to
4+
`out/log/<mode>/<dataset>/<model>/<trainer>/trial_###/cfg.yaml`.
5+
6+
Notes:
7+
- The `<mode>` segment is just a folder namespace (e.g., `sweep`, `experiment`, `sweep_1`).
8+
It does not affect expansion semantics.
59
"""
610

711
from collections.abc import Sequence
@@ -12,6 +16,19 @@
1216
from src.utils.cfg import create_trials, has_cfg_been_run
1317

1418

19+
def discover_modes(contains: str | Sequence[str] | None = None) -> list[str]:
20+
"""Return list of cfg/<mode> folder names, optionally filtered by substring(s)."""
21+
cfg_root = Path('cfg')
22+
modes = [p.name for p in cfg_root.iterdir() if p.is_dir()] if cfg_root.exists() else []
23+
if contains is None:
24+
return modes
25+
if isinstance(contains, str):
26+
key = contains.lower()
27+
return [m for m in modes if key in m.lower()]
28+
keys = {s.lower() for s in contains}
29+
return [m for m in modes if any(k in m.lower() for k in keys)]
30+
31+
1532
def run_trials(
1633
datasets: Sequence[str] | None = None,
1734
modes: Sequence[str] | None = None,
@@ -36,12 +53,12 @@ def run_trials(
3653
if not cfg_root.exists():
3754
return {'trials_total': 0, 'trials_skipped': 0}
3855

39-
# Allowed values and defaults (datasets -> registry keys; modes -> canonical list)
56+
# Allowed datasets come from the registry; modes are discovered dynamically
4057
allowed_datasets = list(DATASETS.keys())
41-
allowed_modes = ['sweep', 'experiment']
58+
discovered_modes = discover_modes()
4259

43-
# Resolve selected modes: default to all; otherwise intersect with allowed
44-
requested_modes: list[str] = allowed_modes if modes is None else [m for m in allowed_modes if m in set(modes)]
60+
# Resolve selected modes: default to all discovered; otherwise intersect
61+
requested_modes: list[str] = discovered_modes if modes is None else [m for m in discovered_modes if m in set(modes)]
4562

4663
# Map selected modes to existing cfg/<mode> directories
4764
mode_dirs = [cfg_root / m for m in requested_modes if (cfg_root / m).exists()]

src/utils/cfg.py

Lines changed: 25 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
from src.models.registry import MODELS
1111
from src.trainers.registry import TRAINERS
1212

13-
# Configuration constants (simplified)
1413
DERIVED_CFG_KEYS = {'input_shape', 'num_classes'}
1514
REQUIRED_KEYS = {
1615
'dataset',
@@ -20,7 +19,7 @@
2019
'base_lr',
2120
'batch_size',
2221
'epochs',
23-
'seed', # a default seed (can be overridden by sweep dimension `seed` list)
22+
'seed',
2423
'num_workers',
2524
}
2625
KEY_ORDER = [
@@ -40,7 +39,6 @@
4039
# Expansion constants
4140
LOGGING_LIST_KEYS = {'batch_metrics'} # list-valued keys not treated as sweep dimensions
4241
HASH_IGNORE_KEYS = DERIVED_CFG_KEYS | LOGGING_LIST_KEYS
43-
SEED_KEY = 'seed' # may be scalar or list (list => sweep over seeds)
4442

4543

4644
@dataclass
@@ -110,7 +108,7 @@ def cfg_hash(cfg: dict) -> str:
110108
configuration, with mapping keys sorted and without ignored keys.
111109
"""
112110
norm = normalize_resolved_cfg(cfg)
113-
data = json.dumps(norm, sort_keys=True, separators=(",", ":"), ensure_ascii=False, allow_nan=False)
111+
data = json.dumps(norm, sort_keys=True, separators=(',', ':'), ensure_ascii=False, allow_nan=False)
114112
return hashlib.sha256(data.encode('utf-8')).hexdigest()
115113

116114

@@ -120,7 +118,8 @@ def _extract_path_info(path: Path) -> tuple[str, str, str, str]:
120118
Expected pattern (after first 'cfg' segment):
121119
cfg/<mode>/<dataset>/<model>/<trainer>/.../cfg.yaml
122120
123-
<mode> must currently be one of {'sweep', 'experiment'}.
121+
The `<mode>` segment is a free-form folder namespace (e.g., 'sweep',
122+
'experiment', 'sweep_1'). Expansion semantics do not depend on this value.
124123
"""
125124
parts = list(path.parts)
126125
try:
@@ -131,17 +130,9 @@ def _extract_path_info(path: Path) -> tuple[str, str, str, str]:
131130
raise ValueError(f'Invalid cfg path (no cfg segment): {path}') from e
132131

133132
tail = parts[cfg_idx + 1 :]
134-
# Legacy directory prefixes ('tune', 'experiments') are no longer supported and not skipped.
135-
# 'sweep' and 'experiment' are treated as real dataset names now, so we do not strip anything.
136-
if tail and tail[0] in {'tune', 'experiments'}:
137-
raise ValueError(
138-
f"Legacy path prefix no longer supported (remove 'tune/' or 'experiments/' from cfg path): {path}"
139-
)
140133
if len(tail) < 4:
141134
raise ValueError(f'Insufficient path depth for mode/dataset/model/trainer: {path}')
142135
mode, dataset, model, trainer = tail[:4]
143-
if mode not in {'sweep', 'experiment'}:
144-
raise ValueError(f"Unsupported mode '{mode}' in cfg path (expected 'sweep' or 'experiment'): {path}")
145136
return mode, dataset, model, Path(trainer).stem
146137

147138

@@ -181,27 +172,17 @@ def _dimension_items(cfg: dict) -> list[tuple[str, list]]:
181172
return dims
182173

183174

184-
def expand_cfg_from_dict(cfg: dict, mode: str) -> list[TrialSpec]:
175+
def expand_cfg_from_dict(cfg: dict) -> list[TrialSpec]:
185176
"""Expand a (possibly merged) cfg dict into trial specs.
186177
187-
For 'sweep' mode: all list-valued keys (except structural allowlist) become
188-
dimensions. 'seed' must be scalar.
189-
For 'experiment' mode: only 'seed' may be list-valued (creating per-seed trials).
178+
Unified behavior (mode-agnostic):
179+
- Every list-valued key at the top level (except those in LOGGING_LIST_KEYS)
180+
is treated as a sweep dimension.
181+
- 'seed' may be scalar or list; if list, it is simply another dimension.
182+
- If no list-valued keys are present, produce a single TrialSpec.
190183
"""
191-
dims: list[tuple[str, list]] = []
192-
for k, v in cfg.items():
193-
if isinstance(v, list) and k not in LOGGING_LIST_KEYS:
194-
if mode == 'sweep':
195-
if k == SEED_KEY:
196-
raise ValueError("sweep cfg.yaml must not contain list-valued 'seed'; provide a single scalar seed")
197-
dims.append((k, v))
198-
elif mode == 'experiment':
199-
if k == SEED_KEY:
200-
dims.append((k, v))
201-
else: # pragma: no cover - defensive, normally enforced earlier
202-
raise ValueError('experiment cfg.yaml may only vary seed; offending list key: ' + k)
203-
else: # pragma: no cover - future mode types
204-
raise ValueError(f'Unsupported mode for expansion: {mode}')
184+
# Identify dimensions using the shared helper for a single source of truth
185+
dims = _dimension_items(cfg)
205186

206187
if not dims:
207188
return [TrialSpec(idx=1, assignments={})]
@@ -215,9 +196,9 @@ def expand_cfg_from_dict(cfg: dict, mode: str) -> list[TrialSpec]:
215196
def create_trials(base_cfg_path: str | Path) -> list[Path]:
216197
"""Write resolved trial cfgs beneath out/log/<mode>/<dataset>/<model>/<trainer>/trial_###/cfg.yaml.
217198
218-
Mode-specific rules:
219-
- sweep: if 'seed' provided it must be scalar (not list); other list hyperparams allowed.
220-
- experiment: only 'seed' may be list-valued (aside from structural allowlist).
199+
Expansion uses unified rules (see `expand_cfg_from_dict`): all list-valued
200+
top-level keys except logging lists become sweep dimensions. Resolved cfgs
201+
must not contain any remaining list-valued hyperparameters.
221202
"""
222203
path = Path(base_cfg_path)
223204
if path.name != 'cfg.yaml':
@@ -234,7 +215,7 @@ def create_trials(base_cfg_path: str | Path) -> list[Path]:
234215
# properly swept. Previously only the leaf cfg file was inspected which
235216
# caused inherited lists (e.g. batch_size) to remain unresolved in the
236217
# written trial cfgs.
237-
trials = expand_cfg_from_dict(merged_base, mode)
218+
trials = expand_cfg_from_dict(merged_base)
238219
# Root output directory for this cfg tree
239220
out_root = Path('out') / 'log' / mode / dataset / model / trainer
240221
out_root.mkdir(parents=True, exist_ok=True)
@@ -269,21 +250,19 @@ def _parse_idx(p: Path) -> int:
269250
resolved = merged_base.copy()
270251
resolved.update(spec.assignments) # apply concrete assignments
271252
# Ensure that any list-valued dimension keys have been resolved to scalars
272-
for k, v in list(resolved.items()):
273-
if isinstance(v, list) and k not in LOGGING_LIST_KEYS:
274-
# If still a list here, it was not selected as a dimension (e.g. leftover due to mode rules)
275-
# For experiment mode only 'seed' may remain list; others are invalid.
276-
if mode == 'experiment' and k == SEED_KEY:
277-
continue
278-
raise ValueError(
279-
f'Unresolved list-valued hyperparameter {k!r} remained in resolved cfg. Check expansion rules.'
280-
)
253+
unresolved_dims = _dimension_items(resolved)
254+
if unresolved_dims:
255+
# With unified expansion, no list-valued hyperparameters should remain.
256+
names = [name for name, _ in unresolved_dims]
257+
raise ValueError(
258+
f'Unresolved list-valued hyperparameters remained in resolved cfg: {names}. Check expansion rules.'
259+
)
281260
# Compute identity hash for de-duplication
282261
new_hash = cfg_hash(resolved)
283262

284263
# If this configuration already exists anywhere under out_root, reuse that path
285264
if new_hash in hash_to_dir:
286-
existing_path = (hash_to_dir[new_hash] / 'cfg.yaml')
265+
existing_path = hash_to_dir[new_hash] / 'cfg.yaml'
287266
written.append(existing_path)
288267
continue
289268

@@ -356,23 +335,14 @@ def get_output_path(cfg_path: str | Path) -> Path:
356335
raise ValueError('Only resolved trial cfg paths are supported.')
357336

358337

359-
def _validate_expansion_rules(_: dict) -> None: # pragma: no cover - retained for potential future rules
360-
return
361-
362-
363338
def has_cfg_been_run(cfg_path: str | Path) -> tuple[bool, str]:
364339
"""A trial is considered run if its cfg exists and batch_log.csv is present."""
365340
try:
366341
cfg_path = Path(cfg_path)
367342
if not cfg_path.exists():
368343
return False, 'Resolved cfg missing'
369-
with cfg_path.open() as f:
370-
resolved = yaml.safe_load(f) or {}
371344
if not (cfg_path.parent / 'batch_log.csv').exists():
372345
return False, 'No batch_log.csv'
373-
# Lightweight validation of registries
374-
if not cfgs_equal(resolved, resolved): # always true, placeholder for future diff
375-
return False, 'Internal mismatch'
376346
return True, 'Outputs present'
377-
except Exception as e: # noqa: BLE001
347+
except Exception as e:
378348
return False, f'Error checking cfg: {e}'
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
from pathlib import Path
2+
3+
from _pytest.monkeypatch import MonkeyPatch
4+
5+
from src.run import run_trials
6+
7+
8+
def test_dynamic_mode_discovery_includes_custom_mode(tmp_path: Path, monkeypatch: MonkeyPatch) -> None:
9+
"""run_trials should discover cfg/<mode> subfolders (e.g., 'sweep_1') and plan runs."""
10+
# Minimal config under a custom mode folder
11+
base_cfg = tmp_path / 'cfg/sweep_1/mnist/simple-conv/jfb/cfg.yaml'
12+
base_cfg.parent.mkdir(parents=True, exist_ok=True)
13+
base_cfg.write_text(
14+
'model: simple-conv\n'
15+
'trainer: jfb\n'
16+
'optimizer: sgd\n'
17+
'batch_size: 2\n'
18+
'epochs: 1\n'
19+
'seed: 0\n'
20+
'num_workers: 0\n'
21+
'base_lr: 0.01\n'
22+
)
23+
24+
# Ensure cwd is temp
25+
cwd = Path.cwd()
26+
monkeypatch.chdir(tmp_path)
27+
28+
# Dry-run planning should count this trial when filtering by the discovered mode
29+
summary = run_trials(datasets=['mnist'], modes=['sweep_1'], dry_run=True)
30+
assert summary['trials_planned'] >= 1
31+
assert summary['trials_run'] == 0
32+
33+
# Without specifying modes, discovered modes should include 'sweep_1'
34+
summary2 = run_trials(datasets=['mnist'], dry_run=True)
35+
assert summary2['trials_planned'] >= 1
36+
assert summary2['trials_run'] == 0
37+
38+
monkeypatch.chdir(cwd)

0 commit comments

Comments
 (0)