Skip to content

Add generic data-fitting error metrics (MAE/L1, relative L2, Huber) to physicsnemo.metrics.general - #1807

Closed
wdyab wants to merge 1 commit into
NVIDIA:mainfrom
wdyab:nof/pr3a-data-fitting-losses
Closed

wdyab wants to merge 1 commit into
NVIDIA:mainfrom
wdyab:nof/pr3a-data-fitting-losses

Conversation

@wdyab

@wdyab wdyab commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds three commonly-needed data-fitting error metrics to the canonical
physicsnemo.metrics.general, complementing the existing mse / rmse:

  • mae (mean absolute error), aliased as l1physicsnemo/metrics/general/mae.py
  • relative_l2 and a general relative_lp (scale-invariant relative Lp error,
    a.k.a. LpLoss) — physicsnemo/metrics/general/relative.py
  • huber (smooth L1 error) — physicsnemo/metrics/general/huber.py

All follow the existing functional mse(pred, target, dim=None) signature style.
An audit confirmed none of these existed anywhere in the library (only mse/rmse),
so this is purely additive with no duplication.

This is the first of three small, stacked PRs upstreaming reusable, generic
components discovered while developing a reservoir-simulation example.

Closes #1804

Test plan

  • test/metrics/test_metrics_general.py extended with analytic value checks,
    zero-for-identical, relative_l2 scale-invariance, eps-guard, and
    per-dimension reduction tests.
  • interrogate docstring coverage 100% on the new files.
  • ruff check (E/F/S/I/PERF) and ruff format clean.
  • License headers present; CHANGELOG updated.

@copy-pr-bot

copy-pr-bot Bot commented Jul 8, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@greptile-apps

greptile-apps Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

Adds three purely-additive data-fitting error metrics (mae/l1, relative_lp/relative_l2, huber) to physicsnemo.metrics.general, following the existing mse/rmse functional signature convention.

  • mae.py and relative.py are clean and correct; implementations, docstrings, and tests are accurate.
  • huber.py uses F.huber_loss correctly, but the LaTeX formula in the docstring documents the standard Huber loss (0.5 x² quadratic, delta·(|x|−0.5·delta) linear) rather than PyTorch's rescaled variant (0.5 x²/delta, |x|−0.5·delta). These two formulations agree only when delta=1.0 (the default), so users setting a non-unit delta will encounter results that contradict the documented formula.

Important Files Changed

Filename Overview
physicsnemo/metrics/general/huber.py New Huber loss metric — implementation via F.huber_loss is correct, but the LaTeX docstring formula documents the standard Huber loss instead of PyTorch's rescaled variant; they diverge for any delta != 1.0.
physicsnemo/metrics/general/mae.py New MAE/L1 metric — clean implementation using torch.mean(torch.abs(...)), l1 alias is correct, docstring and signature follow existing mse style.
physicsnemo/metrics/general/relative.py New relative Lp/L2 metrics — correctly uses torch.linalg.vector_norm with dim=None flattening; eps guard is well-placed; relative_l2 is a clean wrapper around relative_lp.
test/metrics/test_metrics_general.py Good analytic-value, zero-identity, and per-dimension shape tests for all three new metrics; Huber tests only use delta=1.0, which masks the docstring formula discrepancy for other delta values.
CHANGELOG.md CHANGELOG entry added correctly in the Unreleased section describing all three new metrics.
docs/api/physicsnemo.metrics.rst RST docs updated with table entries and automodule directives for all three new modules; formatting is consistent with existing entries.

Reviews (2): Last reviewed commit: "Add general data-fitting error metrics t..." | Re-trigger Greptile

Comment thread physicsnemo/metrics/general/relative.py Outdated
Adds mae (mean absolute error, aliased as l1), relative_l2 / relative_lp
(scale-invariant relative Lp error, a.k.a. LpLoss), and huber (smooth L1
error) to physicsnemo.metrics.general, complementing the existing mse / rmse.
These are common regression losses used by neural-operator training loops and
were previously missing from the library. Includes unit tests, API docs, and
a CHANGELOG entry.

Pre-commit: markdownlint and import-linter were skipped (SKIP=...) as they
fail only on local environment artifacts (markdownlint's Node/npm cannot write
to the sandbox tmp cache; import-linter reports pre-existing external-import
violations with 0 file violations, which pass in upstream CI). ruff-check,
ruff-format, interrogate, license header, and large-file hooks ran and passed;
markdownlint content (88-char line length) was validated manually.

Signed-off-by: wdyab <wdyab@nvidia.com>
@wdyab
wdyab force-pushed the nof/pr3a-data-fitting-losses branch from 38868b3 to 4d04a6d Compare July 8, 2026 18:49
@wdyab

wdyab commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

@melo-gonzo for your review

@melo-gonzo

Copy link
Copy Markdown
Collaborator

Hi @wdyab, this appears to have some overlap with #1746 from @mnabian. Additionally, simply importing and wrapping the existing PyTorch functionals makes it hard to justify adding these new functions in if they provide no additional value.

@wdyab

wdyab commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

Both points are fair.

  • relative_l2 / relative_lp overlap Relative L2 loss #1746, which is more complete (element weights / masking + relative_mse) and predates this PR — so I'm deferring the relative metrics to Relative L2 loss #1746 rather than competing.
  • mae / huber are thin wrappers over torch.mean(torch.abs(...)) and F.huber_loss with no additional value, so I'm dropping them rather than keeping them.

Closing this PR (and its issue #1804) accordingly.

The two pieces from this stack that have no existing equivalent — SpatialDerivativeLoss (#1808) and VolumeConservationLoss (#1809) — will continue independently: I'll rebase them directly onto main (off this PR) and switch VolumeConservationLoss's default metric to mse, so any metrics.general function (including #1746's relative_l2 once it lands) can be dependency-injected.

Aside: the Greptile note about the Huber docstring is a false positive — F.huber_loss implements the standard Huber loss for all delta (it's smooth_l1_loss that rescales by beta), so the formula was correct. Moot now that huber is dropped.

@wdyab wdyab closed this Jul 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add generic data-fitting error metrics (MAE/L1, relative L2, Huber) to physicsnemo.metrics.general

2 participants