Skip to content

Conversation

@Luthaf
Copy link
Member

@Luthaf Luthaf commented Oct 23, 2025

We should reconsider this warning in presence of a mask, because then some of the N will be zero by construction. Let's have a chat before merging!

(This also serves as a PR for me to check that codecov from a fork works again)

Contributor (creator of pull-request) checklist

  • Tests updated (for new features and bugfixes)?
  • Documentation updated (for new features)?
  • Issue referenced (for PRs that solve an issue)?

Maintainer/Reviewer checklist

  • CHANGELOG updated with public API or any other important changes?
  • GPU tests passed (maintainer comment: "cscs-ci run")?

📚 Documentation preview 📚: https://metatrain--856.org.readthedocs.build/en/856/

@Luthaf Luthaf requested a review from jwa7 October 23, 2025 14:04
Copy link
Member

@jwa7 jwa7 left a comment

Choose a reason for hiding this comment

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

After discussion, we can remove the warning as it is not necessary, and replace in the code:

# If any scales are zero or NaN, set them to 1.0
scale_vals_type[scale_vals_type == 0] = 1.0
scale_vals_type[torch.isnan(scale_vals_type)] = 1.0

It would trigger in cases where the data is padded and
then masked in the loss.
@Luthaf Luthaf force-pushed the better-scaler-error branch from 1560f2d to 6bddb01 Compare November 7, 2025 14:56
@Luthaf Luthaf requested a review from jwa7 November 7, 2025 14:56
Copy link
Member

@jwa7 jwa7 left a comment

Choose a reason for hiding this comment

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

Thanks!

@Luthaf
Copy link
Member Author

Luthaf commented Nov 7, 2025

cscs-ci run

@Luthaf Luthaf enabled auto-merge (rebase) November 7, 2025 15:00
@Luthaf Luthaf merged commit 65c459e into metatensor:main Nov 7, 2025
16 checks passed
@Luthaf Luthaf deleted the better-scaler-error branch November 7, 2025 15:19
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.

2 participants