Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Newmetric: NRMSE #2442

Open
wants to merge 25 commits into
base: master
Choose a base branch
from
Open

Newmetric: NRMSE #2442

wants to merge 25 commits into from

Conversation

SkafteNicki
Copy link
Member

@SkafteNicki SkafteNicki commented Mar 9, 2024

What does this PR do?

Fixes #2303

Before submitting
  • Was this discussed/agreed via a Github issue? (no need for typos and docs improvements)
  • Did you read the contributor guideline, Pull Request section?
  • Did you make sure to update the docs?
  • Did you write any new necessary tests?
PR review

Anyone in the community is free to review the PR once the tests have passed.
If we didn't discuss your PR in Github issues there's a high chance it will not be merged.

Did you have fun?

Make sure you had fun coding 🙃


📚 Documentation preview 📚: https://torchmetrics--2442.org.readthedocs.build/en/2442/

@SkafteNicki SkafteNicki added enhancement New feature or request New metric labels Mar 9, 2024
@SkafteNicki SkafteNicki added this to the v1.4.0 milestone Mar 9, 2024
@github-actions github-actions bot added documentation Improvements or additions to documentation topic: Regress labels Mar 9, 2024
@Borda
Copy link
Member

Borda commented May 21, 2024

@SkafteNicki, what is missing here to make it land? 🐿️

@SkafteNicki SkafteNicki marked this pull request as ready for review May 31, 2024 13:35
@SkafteNicki
Copy link
Member Author

@SkafteNicki, what is missing here to make it land? 🐿️

@Borda should be ready now :]

"pearson_corrcoef",
"pearsons_contingency_coefficient",
"pearsons_contingency_coefficient_matrix",
"permutation_invariant_training",
"perplexity",
"pit_permutate",
"precision",
"precision_at_fixed_recall",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shall this be another PR?

@mergify mergify bot added the has conflicts label Jun 4, 2024
@mergify mergify bot removed the has conflicts label Jun 4, 2024
@Borda
Copy link
Member

Borda commented Jun 5, 2024

@SkafteNicki it seems something wrong there is:

>>> import torch
>>> from torchmetrics import NormalizedRootMeanSquaredError
>>> target = torch.tensor([[0.0, 0.0, 0.0], [0.0, 0.0, 0.0]])
>>> preds = torch.tensor([[1.0, 2.0, 3.0], [1.0, 2.0, 3.0]])
>>> nrmse = NormalizedRootMeanSquaredError(num_outputs=3)
>>> nrmse(preds, target)
Expected:
    tensor([1., 1., 1.])
Got:
    tensor([inf, inf, inf])

@Borda
Copy link
Member

Borda commented Jul 22, 2024

@SkafteNicki just a friendly ping :) seems almost done just revisit the examples...

@SkafteNicki
Copy link
Member Author

@SkafteNicki just a friendly ping :) seems almost done just revisit the examples...

yes, just need the final tests to pass :]

sum_squared_error, num_obs = _mean_squared_error_update(preds, target, num_outputs)

target = target.view(-1) if num_outputs == 1 else target
if normalization == "mean":

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please could we also have an L2 option which normalises by the sum of squares of target? This is a commonly used NMSE in image processing eg for MRI (eg see Eq. 5 in https://www.ncbi.nlm.nih.gov/pmc/articles/PMC3947978/#sec-a.w.btitle )

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @Borda , do you need any more input from me regarding this? Thanks

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Borda Borda requested review from Borda and Andrewwango August 2, 2024 15:42
@Borda Borda force-pushed the master branch 2 times, most recently from d0a5568 to 9fc79ae Compare September 16, 2024 08:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request New metric topic: Regress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Normalized Root Mean Squared Error (NRMSE) Calculation
4 participants