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

Explicitly use the linalg.vector_norm call in comm/ #6960

Closed
wants to merge 5 commits into from

Conversation

loadams
Copy link
Contributor

@loadams loadams commented Jan 17, 2025

github-merge-queue bot pushed a commit that referenced this pull request Jan 21, 2025
…orm` (#6931)

- [x] Update PR since `torch.norm` and `torch.linalg.norm` have
[different function
signatures](https://pytorch.org/docs/stable/generated/torch.linalg.norm.html#torch.linalg.norm).
- [x] Check if there are any numeric differences between the functions.
- [x] Determine why there appear to be performance differences from
others [here](pytorch/pytorch#136360).
- [x] Update to `torch.linalg.vectornorm`
Follow up PR handles these in the comm folder: #6960
@loadams loadams requested review from tjruwase and tohtana January 21, 2025 18:29
@loadams loadams enabled auto-merge January 21, 2025 18:56
@loadams loadams disabled auto-merge January 21, 2025 19:11
@GuanhuaWang
Copy link
Member

GuanhuaWang commented Jan 21, 2025

Hi @loadams
I may misunderstood, is there a reason for switching from norm to vec_norm? To me, norm seems to cover the functionality of vec_norm?

@loadams
Copy link
Contributor Author

loadams commented Jan 22, 2025

Hi @loadams I may misunderstood, is there a reason for switching from norm to vec_norm? To me, norm seems to cover the functionality of vec_norm?

@GuanhuaWang - this is from the pytorch docs, it was mostly to make it more clear, but we don't "need" to update these like we needed to update from torch.norm to torch.linalg.norm

image

@loadams loadams closed this Jan 22, 2025
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