fix(calibration): score the differentiable loss on the calibration period - #7
Merged
Conversation
…riod kge_loss and nse_loss dropped warmup and then scored everything that remained. With a calibration/evaluation split configured, that span covers both windows, so gradient-based optimizers were trained on the held-out evaluation period — the split-sample test was not honest — and the score they reported was computed over a different window than the Calib_* metrics written at final evaluation. Same defect as jTOPMODEL, jSAC-SMA and jXAJ. Thread a calibration slice through both losses, both backends, and the gradient-function factories; the worker supplies it from the shared InMemoryModelWorker.get_calibration_slice(). Verified on the P3 Bow-at-Banff ensemble: ADAM reports best 0.9010 == Calib_KGE 0.9010, with Eval_KGE 0.7946 genuinely held out. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Guards the leakage fix directly: a cal_slice covering the whole post-warmup record must be a no-op, a narrower slice must change the score, and two disjoint windows must not collapse to the same number. Verified these fail against the pre-fix behaviour — disabling the slice in _eval_window fails 4 of the 6 cases, so an accidental revert cannot pass silently. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
kge_lossandnse_lossdropped warmup and then scored everything that remained. With a calibration/evaluation split configured, that span covers both windows.Two consequences:
Calib_*metrics written at final evaluation.HEC-HMS had no completed ADAM run in the P3 ensemble, so this is fixed before it could produce misleading numbers. First run on fixed code: best 0.9010 == Calib_KGE 0.9010, with Eval_KGE 0.7946 genuinely held out.
Every other algorithm in the ensemble agreed exactly, because they score through
_run_simulationrather than the JAX loss. L-BFGS was unaffected only because its configs request finite-difference gradients.Fix
Thread a calibration slice through both losses, both backends, and the gradient-function factories. The worker supplies it from the shared
InMemoryModelWorker.get_calibration_slice(), so all packages share one implementation. Requires the companion SYMFLUENCE PR.Tests
New
test_calibration_window.py(6 tests): a slice covering the whole post-warmup record is a no-op, a narrower slice changes the score, and two disjoint windows do not collapse to the same number.These were mutation-tested — disabling the slice in
_eval_windowfails 4 of the 6 — so an accidental revert cannot pass silently.Same defect found and fixed across jHBV, jTOPMODEL, jSAC-SMA, jXAJ, jHEC-HMS, jFUSE and cFUSE.
🤖 Generated with Claude Code