Give the cond_cat/cond_cont column layout one owner (gitea #37)
CI / Lint (ruff check) (push) Successful in 30s
CI / Format (ruff format) (push) Successful in 30s
CI / Sync project version with tag (push) Has been skipped
CI / Type check (ty) (push) Successful in 43s
CI / Lint (ruff check) (pull_request) Successful in 40s
CI / Format (ruff format) (pull_request) Successful in 43s
CI / Sync project version with tag (pull_request) Has been skipped
CI / Type check (ty) (pull_request) Successful in 45s
CI / Tests (pull_request) Successful in 3m24s
CI / Tests (push) Successful in 3m33s
CI / Lint (ruff check) (push) Successful in 30s
CI / Format (ruff format) (push) Successful in 30s
CI / Sync project version with tag (push) Has been skipped
CI / Type check (ty) (push) Successful in 43s
CI / Lint (ruff check) (pull_request) Successful in 40s
CI / Format (ruff format) (pull_request) Successful in 43s
CI / Sync project version with tag (pull_request) Has been skipped
CI / Type check (ty) (pull_request) Successful in 45s
CI / Tests (pull_request) Successful in 3m24s
CI / Tests (push) Successful in 3m33s
The conditioning arrays' column order was written down three times — twice
in giant/data/transforms.py (build_cond_features and build_features each
built cond_cont and cond_cat from scratch) and again in
giant/model/encoders.py (cat_col_layout, plus hand-written
COND_DIM_BASE + PARTICLE_PHYS_DIM slicing in ConditionEncoder). The three
were held in sync only by parallel comments, so a wrong column order
produced silently mis-indexed features rather than an exception.
The drift had already happened, twice, both times in build_features:
- 5b63dfd added per-axis vocab-lookup strictness (an out-of-vocab
pdg/material must not KeyError under "physical"/"onehot", where the
index is never read) to build_cond_features only.
- _cond_normalizer_transform's legacy-normalizer padding, which keeps a
pre-physical-conditioning 8-wide cond normalizer loadable, was likewise
only wired into build_cond_features — so `giant predict` on such a
checkpoint died with a broadcast error.
New giant/cond_layout.py holds a frozen CondLayout built from the
(particle, material) mode pair, exposing named cond_cont slices
(base/particle_phys/material_phys) and cond_cat columns
(PDG_COL/MAT_COL/particle_topn_col/material_topn_col/cat_dim). Both
builders now share one _build_cond_arrays, ConditionEncoder reads its
slices off the same object, and PdgRouter/ProcessRouter use the named
dense-vocab columns instead of literal 0/1. CondLayout also absorbs the
two duplicated axis-type validations, keeping their message text verbatim.
Decisions taken while planning:
- Scope is CondLayout only. The issue's second half — a
CONDITIONING_AXIS_REGISTRY registering (feature_columns, encoder_module)
as a pair — is deferred: it would force ConditioningConfig's fixed
particle/material fields into a dynamic axis map and ripple through
pipeline.py, checkpoint_io.py and rollout.py, i.e. a config-schema break
with no consumer yet.
- The two divergences above are unified onto build_cond_features'
behaviour rather than preserved as parameters, so the new single source
of truth doesn't carry the old split forward. Each gets a regression
test that fails before this commit.
- cat_col_layout is replaced outright (deleted, dropped from network.py's
__all__, its four tests rewritten against CondLayout) rather than kept
as a wrapper — two spellings of the same fact is the defect itself.
cond_cat's width is now the layout's call rather than "did the caller pass
a map", so an "onehot" axis without its top-N map raises instead of
yielding a narrower array that ConditionEncoder would index out of bounds.
pipeline.py's normalizer-fitting pass reads only cond_cont but had to be
handed the maps to satisfy that.
No parameter, buffer or state_dict change; existing checkpoints load
unchanged, and the protected migration surfaces are untouched.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -2,6 +2,7 @@ import warnings
|
||||
|
||||
import numpy as np
|
||||
import pytest
|
||||
from giant.cond_layout import AXIS_TYPES, CondLayout
|
||||
from giant.constants import COND_DIM, COND_DIM_BASE, K_MAX
|
||||
from giant.data.transforms import (
|
||||
build_cond_features,
|
||||
@@ -531,6 +532,129 @@ def test_build_cond_features_rejects_legacy_normalizer_in_physical_mode(
|
||||
)
|
||||
|
||||
|
||||
# ── build_cond_features / build_features share one column layout (gitea #37) ──
|
||||
|
||||
|
||||
@pytest.mark.parametrize("particle_type", AXIS_TYPES)
|
||||
@pytest.mark.parametrize("material_type", AXIS_TYPES)
|
||||
def test_both_builders_agree_column_for_column(particle_type, material_type, fake_material_props):
|
||||
"""The two builders used to lay out cond_cont/cond_cat independently and
|
||||
drift apart silently. They now share `_build_cond_arrays`, so for every
|
||||
mode pair they must produce identical arrays."""
|
||||
data = _minimal_step_data(3)
|
||||
pdg_map, mat_map = {11: 0}, {"PbWO4": 0}
|
||||
pdg_topn = {11: 0} if particle_type == "onehot" else None
|
||||
mat_topn = {"PbWO4": 0} if material_type == "onehot" else None
|
||||
|
||||
cond_cont, cond_cat = build_cond_features(
|
||||
data,
|
||||
pdg_map,
|
||||
mat_map,
|
||||
particle_conditioning=particle_type,
|
||||
material_conditioning=material_type,
|
||||
pdg_topn_map=pdg_topn,
|
||||
mat_topn_map=mat_topn,
|
||||
)
|
||||
feats = build_features(
|
||||
data,
|
||||
pdg_map,
|
||||
mat_map,
|
||||
particle_conditioning=particle_type,
|
||||
material_conditioning=material_type,
|
||||
pdg_topn_map=pdg_topn,
|
||||
mat_topn_map=mat_topn,
|
||||
)
|
||||
|
||||
layout = CondLayout.from_types(particle_type, material_type)
|
||||
assert cond_cat.shape[1] == layout.cat_dim
|
||||
np.testing.assert_array_equal(feats.cond_cont, cond_cont)
|
||||
np.testing.assert_array_equal(feats.cond_cat, cond_cat)
|
||||
|
||||
|
||||
def test_build_features_physical_mode_tolerates_out_of_vocab_pdg_and_material():
|
||||
"""The permissive vocab lookup added for "physical"/"onehot" mode (see
|
||||
build_cond_features) applies to build_features too — `giant predict` on a
|
||||
file whose pdg/material aren't in the checkpoint's dense vocab must not
|
||||
KeyError when nothing reads those indices."""
|
||||
pdg_map = {11: 0, 22: 1}
|
||||
mat_map = {"G4_AIR": 0}
|
||||
data = _minimal_step_data(2)
|
||||
data["pdg"] = np.full(2, 13, dtype=np.int64) # not in pdg_map
|
||||
data["material"] = np.full(2, "G4_Pb", dtype=object) # not in mat_map
|
||||
|
||||
_, cond_cat, *_ = build_features(
|
||||
data,
|
||||
pdg_map,
|
||||
mat_map,
|
||||
particle_conditioning="physical",
|
||||
material_conditioning="physical",
|
||||
)
|
||||
np.testing.assert_array_equal(cond_cat, [[0, 0], [0, 0]]) # dummy indices, no raise
|
||||
|
||||
with pytest.raises(KeyError):
|
||||
build_features(
|
||||
data,
|
||||
pdg_map,
|
||||
mat_map,
|
||||
particle_conditioning="embedding",
|
||||
material_conditioning="embedding",
|
||||
)
|
||||
|
||||
|
||||
def test_build_features_pads_legacy_normalizer_in_embedding_mode():
|
||||
"""The legacy-normalizer padding (a pre-physical-conditioning checkpoint's
|
||||
cond normalizer is COND_DIM_BASE wide) applies to build_features too —
|
||||
`giant predict` reaches build_features, not build_cond_features."""
|
||||
data = _minimal_step_data(3)
|
||||
pdg_map, mat_map = {11: 0}, {"PbWO4": 0}
|
||||
legacy_norm = Normalizer()
|
||||
legacy_norm.mean = np.zeros(COND_DIM_BASE, dtype=np.float32)
|
||||
legacy_norm.std = np.ones(COND_DIM_BASE, dtype=np.float32)
|
||||
|
||||
cond_cont, *_ = build_features(
|
||||
data,
|
||||
pdg_map,
|
||||
mat_map,
|
||||
cond_normalizer=legacy_norm,
|
||||
particle_conditioning="embedding",
|
||||
material_conditioning="embedding",
|
||||
)
|
||||
|
||||
assert cond_cont.shape[-1] == COND_DIM
|
||||
np.testing.assert_allclose(cond_cont[:, COND_DIM_BASE:], 0.0)
|
||||
|
||||
|
||||
def test_build_features_rejects_legacy_normalizer_in_physical_mode(fake_material_props):
|
||||
data = _minimal_step_data(3)
|
||||
pdg_map, mat_map = {11: 0}, {"PbWO4": 0}
|
||||
legacy_norm = Normalizer()
|
||||
legacy_norm.mean = np.zeros(COND_DIM_BASE, dtype=np.float32)
|
||||
legacy_norm.std = np.ones(COND_DIM_BASE, dtype=np.float32)
|
||||
|
||||
with pytest.raises(ValueError, match="predates physical-property conditioning"):
|
||||
build_features(
|
||||
data,
|
||||
pdg_map,
|
||||
mat_map,
|
||||
cond_normalizer=legacy_norm,
|
||||
particle_conditioning="physical",
|
||||
material_conditioning="physical",
|
||||
)
|
||||
|
||||
|
||||
def test_onehot_axis_without_its_topn_map_raises():
|
||||
"""`cond_cat`'s width is the layout's call, so a "onehot" axis with no
|
||||
top-N map is a hard error rather than a silently-narrower array that
|
||||
ConditionEncoder would then index out of bounds."""
|
||||
data = _minimal_step_data(2)
|
||||
pdg_map, mat_map = {11: 0}, {"PbWO4": 0}
|
||||
|
||||
with pytest.raises(ValueError, match="needs pdg_topn_map"):
|
||||
build_cond_features(data, pdg_map, mat_map, particle_conditioning="onehot")
|
||||
with pytest.raises(ValueError, match="needs mat_topn_map"):
|
||||
build_cond_features(data, pdg_map, mat_map, material_conditioning="onehot")
|
||||
|
||||
|
||||
# ── sorted_membership / _vectorized_map_lookup ──────────────────────────────
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user