Share the strength GP fit across tests: 399s -> 135s

Makefile

@@ -85,6 +85,19 @@ format:
 # --- Python unit tests ---------------------------------------------
 # Mirrors .github/workflows/tests.yml :test. We drop --cov-report=xml
 # because we don't need the coverage.xml artefact locally.
+# Serial on purpose. test/shared_fits.py fits the production strength GP once
+# per process and hands out deepcopies, which took this target from ~399 s to
+# ~135 s (405/393 -> 141/130, same machine, coverage on).
+#
+# Parallelism was tried and removed. pytest-xdist did help BEFORE the fits were
+# shared (435 s -> 200 s, measured on a busier machine than the 399 s figure
+# above -- the two baselines are not directly comparable), but afterwards it
+# stopped paying for itself -- 135 s
+# serial against ~184 s at -n 4, since workers are separate processes that each
+# re-import torch and, without grouping, each refit. It also carried a sharp
+# edge: with the default --dist load the shared fit scatters across workers and
+# the suite measured 265 s, slower than not parallelising at all. Recorded here
+# so it is not rediscovered.
 test-py:
 	$(PYTHON) -m pytest test/ -v --tb=short --cov=boxcrete --cov-report=term-missing --cov-fail-under=100
 

test/shared_fits.py

@@ -0,0 +1,104 @@
+"""Copy-on-access providers for the expensive shared GP fits.
+
+Four tests across three modules each need the production strength GP, and
+each was fitting its own copy:
+
+    108.6s  setup  test_models.py::TestPredictiveQualityRegression
+    108.3s  setup  test_models.py::TestGetModelListWithCost
+    106.9s  call   test_lengthscale_identifiability
+    102.9s  call   test_strength_curve_monotonicity
+
+They are the *same* fit. ``SustainableConcreteModel.fit_strength_model``
+calls ``fit_strength_gp(X, Y, Yvar, X_bounds)`` on ``data.strength_data``,
+which is exactly what the other two call directly; ``fit_strength_gp``
+defaults to ``seed=0`` and applies it via ``torch.manual_seed``, and every
+caller uses that default. ``DATA_PATH`` is the default path for
+``load_concrete_strength``, so the inputs match too. One fit therefore
+serves all four.
+
+Isolation:
+
+* The fitted objects live in a closure. No module-level name is bound to
+  them, so there is no reference a caller reaches for by accident, and
+  every accessor returns ``copy.deepcopy``. This is a strong convention,
+  not a hard guarantee -- the cache is still reachable in two hops via
+  ``get_fitted_strength_gp.__closure__``, which takes deliberate effort.
+* Measured, not assumed: mutating a copy's parameters leaves the original
+  untouched, and the copy costs ~0.00s against a ~100s fit. Independence
+  was also checked structurally -- zero shared tensor identities and zero
+  shared storage between a copy and the original.
+* Note ``fit_strength_model`` returns the model in EVAL mode (it computes
+  a diagnostic MLL under ``torch.no_grad`` at the end of the fit), so a
+  consumer calling ``.eval()`` is a no-op on state the original already
+  has. Mode changes on a copy do not reach the original either way.
+* ``copy.deepcopy`` drops GPyTorch's ``prediction_strategy`` (its
+  ``__deepcopy__`` deliberately returns ``None``), so each copy rebuilds
+  its posterior caches lazily. That is why copies cannot inherit or
+  corrupt cached solves; posteriors were verified bit-identical to the
+  original's.
+
+The cache is per process, which is all that is needed: the suite runs
+serially. Parallelism was evaluated and dropped -- see the note above
+``test-py`` in the Makefile.
+"""
+
+from __future__ import annotations
+
+import copy
+
+from boxcrete.concrete_model import SustainableConcreteModel
+from boxcrete.utils import load_concrete_strength
+
+__all__ = ["get_fitted_concrete_model", "get_fitted_strength_gp"]
+
+
+def _make_providers():
+    """Build the accessors over a closure-private cache.
+
+    Deliberately a factory: the fitted model is reachable only from inside
+    this scope, so callers physically cannot obtain the shared instance.
+    """
+    cache: dict = {}
+
+    def _ensure() -> dict:
+        if not cache:
+            data = load_concrete_strength()
+            model = SustainableConcreteModel(strength_days=[1, 28])
+            # Free -- constructed from coefficients, no optimisation.
+            model.fit_gwp_model(data)
+            # The single expensive fit. Called through the public method
+            # rather than fit_strength_gp directly, so the production code
+            # path under test is still the one exercised.
+            model.fit_strength_model(data)
+            cache["data"] = data
+            cache["model"] = model
+        return cache
+
+    def get_fitted_concrete_model():
+        """A fitted ``SustainableConcreteModel`` and its dataset.
+
+        Both are fresh deepcopies; mutate them freely.
+        """
+        c = _ensure()
+        return copy.deepcopy(c["model"]), copy.deepcopy(c["data"])
+
+    def get_fitted_strength_gp():
+        """``(gp, X, Y, Yvar, X_bounds)`` for the production strength GP.
+
+        Equivalent to calling ``fit_strength_gp`` directly with the default
+        seed; the GP is the one that method produced. Fresh deepcopy.
+        """
+        c = _ensure()
+        X, Y, Yvar, X_bounds = c["data"].strength_data
+        return (
+            copy.deepcopy(c["model"].strength_model),
+            X.clone(),
+            Y.clone(),
+            Yvar.clone() if Yvar is not None else None,
+            X_bounds.clone() if X_bounds is not None else None,
+        )
+
+    return get_fitted_concrete_model, get_fitted_strength_gp
+
+
+get_fitted_concrete_model, get_fitted_strength_gp = _make_providers()

test/test_lengthscale_identifiability.py

@@ -63,6 +63,7 @@
     _default_lengthscale_prior,
 )
 from boxcrete.utils import DEFAULT_X_COLUMNS, REPO_DIR, load_concrete_strength
+from .shared_fits import get_fitted_strength_gp
 
 STRENGTH_JSON_PATH = os.path.join(REPO_DIR, "docs", "model", "strength.json")
 TEST_VECTORS_PATH = os.path.join(REPO_DIR, "docs", "model", "test_vectors.json")
@@ -106,20 +107,17 @@ def _load_strength_params():
         return json.load(f)
 
 
-@lru_cache(maxsize=1)
 def _fit_default_strength_gp():
-    """Fit the production strength GP once (with the default within-group
-    shrinkage prior) and cache it across tests. Saves ~10s per extra test."""
-    torch.manual_seed(0)
-    data = load_concrete_strength()
-    X, Y, Yvar, X_bounds = data.strength_data
-    gp = fit_strength_gp(
-        X=X,
-        Y=Y,
-        Yvar=Yvar,
-        X_bounds=X_bounds,
-    )
-    return gp, X, Y, Yvar, X_bounds
+    """The production strength GP with the default within-group shrinkage
+    prior, plus its training tensors.
+
+    Delegates to the cross-module provider (test/shared_fits.py) rather
+    than fitting here: three other tests need this identical fit, and it
+    costs ~105s. The provider hands back a fresh deepcopy each call, so
+    the previous module-local lru_cache is no longer needed -- and callers
+    can now mutate the model without affecting siblings.
+    """
+    return get_fitted_strength_gp()
 
 
 @lru_cache(maxsize=1)

test/test_models.py

@@ -17,6 +17,7 @@
 from boxcrete.slump_model import fit_slump_gp
 from boxcrete.strength_model_legacy import get_strength_gp_input_transform
 from boxcrete.utils import DATA_PATH, load_concrete_strength, SLUMP_Y_COLUMNS
+from .shared_fits import get_fitted_concrete_model
 from parameterized import parameterized
 
 # Limit optimizer iterations in tests for speed (follows BoTorch testing convention)
@@ -380,11 +381,9 @@ class TestPredictiveQualityRegression(unittest.TestCase):
     @classmethod
     def setUpClass(cls):
         super().setUpClass()
-        torch.manual_seed(42)
-        cls.data = load_concrete_strength(data_path=DATA_PATH)
-        cls.shared_model = SustainableConcreteModel(strength_days=[1, 28])
-        cls.shared_model.fit_gwp_model(cls.data)
-        cls.shared_model.fit_strength_model(cls.data)
+        # Shared with three other modules that need the identical fit; see
+        # test/shared_fits.py. Returns a fresh deepcopy every call.
+        cls.shared_model, cls.data = get_fitted_concrete_model()
 
         # Slump uses a separate dataset shape (different Y_columns).
         torch.manual_seed(42)
@@ -540,11 +539,8 @@ class TestGetModelListWithCost(unittest.TestCase):
     @classmethod
     def setUpClass(cls):
         super().setUpClass()
-        torch.manual_seed(42)
-        cls.shared_data = load_concrete_strength(data_path=DATA_PATH)
-        cls.shared_base_model = SustainableConcreteModel(strength_days=[1, 28])
-        cls.shared_base_model.fit_gwp_model(cls.shared_data)
-        cls.shared_base_model.fit_strength_model(cls.shared_data)
+        # Shared across modules; see test/shared_fits.py.
+        cls.shared_base_model, cls.shared_data = get_fitted_concrete_model()
 
     def setUp(self):
         import copy

test/test_strength_curve_monotonicity.py

@@ -50,7 +50,7 @@
 REPO_ROOT = Path(__file__).resolve().parent.parent
 sys.path.insert(0, str(REPO_ROOT / "experiments"))
 
-from boxcrete.utils import load_concrete_strength  # noqa: E402
+from .shared_fits import get_fitted_strength_gp  # noqa: E402
 
 STRENGTH_JSON_PATH = REPO_ROOT / "docs" / "model" / "strength.json"
 COMPOSITIONS_JSON_PATH = REPO_ROOT / "docs" / "model" / "compositions.json"
@@ -147,11 +147,11 @@ def _predict_grid_for_committed_model() -> np.ndarray:
 def _predict_grid_for_fresh_fit() -> np.ndarray:
     """Fit the V2 strength GP via the public ``boxcrete.fit_strength_gp``
     and predict on the dense ``[n_comp, N_TIMES]`` grid."""
-    from boxcrete import fit_strength_gp
-
-    data = load_concrete_strength()
-    X, Y, Yvar, bounds = data.strength_data
-    model = fit_strength_gp(X=X, Y=Y, Yvar=Yvar, X_bounds=bounds, seed=0)
+    # Shared with three other modules; see test/shared_fits.py. Equivalent
+    # to fit_strength_gp(..., seed=0) -- that is exactly how the provider
+    # produces it -- but fitted once per worker instead of per module.
+    # The returned model is a fresh deepcopy, so .eval() below cannot leak.
+    model, X, Y, Yvar, bounds = get_fitted_strength_gp()
     model.eval()
 
     compositions = json.loads(COMPOSITIONS_JSON_PATH.read_text())["compositions"]