Skip to content

Commit 1cf287a

Browse files
Strengthen do_approx_sphere regression guard (#146)
The pr-test-analyzer review found that test_end_to_end_correlation_vs_fortran_from_sample_params_json's correlation/Amari thresholds at max_iter=100 don't reliably catch a reintroduced do_approx_sphere=false: measured 0.9986 correlation and 0.0049 Amari with the bug present, comfortably passing, only marginally failing by max_iter=500. Add a direct, deterministic assertion on the resolved do_approx_sphere value (independent of optimization dynamics) as the actual regression guard, and bump max_iter to 300 so the fit-based checks stay a meaningful secondary sanity check.
1 parent 7949329 commit 1cf287a

1 file changed

Lines changed: 25 additions & 1 deletion

File tree

pyAMICA/tests/torch_tests/test_ng_backend.py

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1134,12 +1134,23 @@ def test_end_to_end_correlation_vs_fortran_from_sample_params_json():
11341134
``do_approx_sphere``) silently overrides that constructor's safe default
11351135
when loaded through this path, but never touches ``_fresh_ng``'s explicit
11361136
kwargs above, so it can regress here while the other test stays green.
1137+
1138+
The end-to-end correlation/Amari checks below are too loose on their own
1139+
to reliably catch this at a practical iteration budget: a reintroduced
1140+
``do_approx_sphere: false`` still clears both thresholds at ``max_iter``
1141+
below a few hundred (issue #144 PR review measured 0.9986 correlation /
1142+
0.0049 Amari at 100 iterations, only marginally failing by 500). The
1143+
explicit ``do_approx_sphere`` assertion below is the actual regression
1144+
guard -- deterministic, independent of optimization dynamics -- and the
1145+
fit-based checks stay as a broader "does the JSON-loaded pipeline work"
1146+
sanity check.
11371147
"""
11381148
import sys
11391149

11401150
root = Path(__file__).resolve().parents[2].parent
11411151
sys.path.insert(0, str(root))
11421152
from validate_implementations import (
1153+
_NG_PARAMS,
11431154
load_sample_data,
11441155
run_fortran_amica,
11451156
run_pytorch_amica,
@@ -1149,7 +1160,20 @@ def test_end_to_end_correlation_vs_fortran_from_sample_params_json():
11491160

11501161
data, params = load_sample_data()
11511162
params = dict(params)
1152-
params["max_iter"] = 100
1163+
params["max_iter"] = 300
1164+
1165+
assert "do_approx_sphere" in _NG_PARAMS, (
1166+
"AMICATorchNG must accept do_approx_sphere for this guard to mean "
1167+
"anything -- update this test if the constructor signature changed"
1168+
)
1169+
assert params.get("do_approx_sphere") is True, (
1170+
"sample_params.json's do_approx_sphere maps straight through to "
1171+
"AMICATorchNG's constructor kwarg of the same name (run_pytorch_amica "
1172+
"passes through any params.json key that is in _NG_PARAMS unchanged); "
1173+
"False here silently breaks Fortran parity (non-symmetric PCA "
1174+
"whitening instead of symmetric ZCA sphering)"
1175+
)
1176+
11531177
out = root / "pyAMICA" / "tests" / "torch_tests" / "_ng_e2e_json_tmp"
11541178
out.mkdir(parents=True, exist_ok=True)
11551179
fortran = run_fortran_amica(data, params, out, SEED)

0 commit comments

Comments
 (0)