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

Fix support for torch.var_mean. #8275

Merged
merged 3 commits into from
Oct 18, 2024
Merged

Fix support for torch.var_mean. #8275

merged 3 commits into from
Oct 18, 2024

Conversation

mrguenther
Copy link
Contributor

The default argument correction=None did not match the Torch API (default 1) and wasn't supported by the Jax API (where it's not nullable). Changing the default to 1 fixed the failing test case.

Issue: #7542

The default argument `correction=None` did not match the Torch API
(default 1) and wasn't supported by the Jax API (where it's not
nullable). Changing the default to 1 fixed the failing test case.

Issue: pytorch#7542
@mrguenther
Copy link
Contributor Author

mrguenther commented Oct 17, 2024

To clarify, correction=None does match the internal function's default argument, but it doesn't match the documented public API of torch.var_mean. I assume Torch internally replaces None with 1 by default.

Actually, I should probably add the safety check

if correction is None:
  correction = 1

inside the function definition. That way, it will still work if Torch ever changes its implementation such that correction=None is explicitly passed into the internal function.

mrguenther added 2 commits October 17, 2024 13:51
Torch doesn't ever explicitly pass `correction=None` into the
implementation of `var_mean`, but the API does technically specify that
it's nullable. Therefore, we add a null check just to be safe.

Issue: pytorch#7542
For consistency with the following `var` call, and because `axis` is
officially a keyword argument, change the `mean` call to pass `axis=dim`
as a keyword argument instead of a positional argument.

Issue: pytorch#7542
@mrguenther
Copy link
Contributor Author

I added that None check per my above comment.

@qihqi qihqi merged commit 2d73a5f into pytorch:master Oct 18, 2024
3 checks passed
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