fix(training): pass (y_true, y_pred) to fit metrics and detach them to host - #456
Open
ChrisW09 wants to merge 1 commit into
Open
fix(training): pass (y_true, y_pred) to fit metrics and detach them to host#456ChrisW09 wants to merge 1 commit into
ChrisW09 wants to merge 1 commit into
Conversation
…o host
Two defects at the same two call sites made fit(train_metrics=/val_metrics=)
unusable and silently wrong:
- Metrics were invoked as metric_fn(preds, labels), but DeepTabMetric is
defined and documented as __call__(y_true, y_pred). Every asymmetric
metric therefore logged a wrong number (R2Score 0.708 correct vs 0.523
swapped), and SklearnBaseLSS.evaluate() -- which calls the same objects
the right way round -- disagreed with the val_* metric logged during fit
for the same model and data.
- The shipped metrics are numpy-based, but the call sites handed them live
torch tensors: train_metrics raised 'Can't call numpy() on Tensor that
requires grad' even on CPU, and val_metrics raised 'can't convert mps:0
device type tensor to numpy' on the default accelerator. The documented
fit(..., val_metrics={'mae': MeanAbsoluteError()}) usage could not run.
Fixes #443
Co-Authored-By: Claude Fable 5 <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
1 participant
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.
Fixes #443
Problems
Two defects at the same two call sites (
lightning_module.py:420,:476):metric_fn(preds, labels), butDeepTabMetric.__call__is documented as(y_true, y_pred). Confirmed with a probe on an LSS model:y_truereceived(8, 2)— the two Normal parameters — andy_predreceived(8, 1), the targets. Asymmetric metrics logged wrong numbers (R2Score0.708 correct vs 0.523 swapped).SklearnBaseLSS.evaluate()calls the same objects the correct way round, soval_crpsduringfitandcrpsfromevaluate()disagreed for the same model and data.deeptab.metricsis numpy-based, but the call sites passed live torch tensors:RuntimeError: Can't call numpy() on Tensor that requires gradfortrain_metricseven on CPU, andTypeError: can't convert mps:0 device type tensor to numpyforval_metricson the default accelerator. So the usage documented indeeptab/metrics/__init__.py,deeptab/metrics/base.pyanddocs/api/metrics/index.rstcould not run at all.Fix
Call
metric_fn(targets, preds)in the documented order, with.detach().cpu()applied to both before handing them to the numpy-based metrics.needs_rawhandling is unchanged.Tests
New
tests/test_fit_metrics_wiring.py: a probe metric asserts the targets arrive asy_trueand the distribution parameters asy_pred, plusMeanAbsoluteErrorused as both a train and a val metric. Verified 2 of the 3 fail onmain(the third fails on a GPU/MPS runner).tests/test_metrics.py,test_observability.pyandtest_lss_base.pypass unchanged.🤖 Generated with Claude Code