Skip to content

torch.linalg.vector_norm : Set ord default as specified in docs #1383

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

Merged

Conversation

MiWeiss
Copy link
Contributor

@MiWeiss MiWeiss commented Oct 16, 2024

The docs of this method specify:

<param name="ord">Order of norm. Default: 2</param>

Said default was however not actually implemented

@MiWeiss
Copy link
Contributor Author

MiWeiss commented Oct 16, 2024

@dotnet-policy-service agree company="Digitec Galaxus"

@MiWeiss
Copy link
Contributor Author

MiWeiss commented Oct 16, 2024

Failing test looks - at first glance - not to be related to the commited change.

@yueyinqiu
Copy link
Contributor

this test also failed in #1376 , but it doesn't occur on my machine, a bit strange.

@MiWeiss
Copy link
Contributor Author

MiWeiss commented Oct 16, 2024

Maybe because you're not using Windows-NetCore locally? It only fails for that...

image

@yueyinqiu
Copy link
Contributor

hmm... i've tried all the possible combinations, win+core+cpu, win+core+gpu, win+framework+cpu, win+framework+gpu

by the way it also failed on other platforms in #1376

perhaps it's related to the gpu or cuda version, and does not appear every time

Copy link
Contributor

@yueyinqiu yueyinqiu left a comment

Choose a reason for hiding this comment

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

by the way you might also edit REALEASENOTES.md for this

@MiWeiss
Copy link
Contributor Author

MiWeiss commented Oct 17, 2024

by the way you might also edit REALEASENOTES.md for this

Sure. Just pushed the adapted releasenotes. I assumed the next release is going to be a patch, and classified the change as bug.

@NiklasGustafsson
Copy link
Contributor

@MiWeiss -- thanks for the contribution. I'll take a look later today.

@NiklasGustafsson
Copy link
Contributor

The next release will be a patch.

I'm waiting for some more engineering resources to start helping out with TorchSharp, and then I'll use a couple of bugs as concrete work items to help introduce them to the code base. If this is essential, we can always do a quick release before then.

@NiklasGustafsson
Copy link
Contributor

Failing test looks - at first glance - not to be related to the commited change.

There are some CI/CD instabilities. The machines we use to build with are inexpensive and often have trouble with resourcing. I rerun jobs all the time, and things usually go better with no changes to the PRs.

@NiklasGustafsson NiklasGustafsson merged commit 538f2e3 into dotnet:main Oct 18, 2024
2 checks passed
@NiklasGustafsson
Copy link
Contributor

The merged build is failing because of some error in package signing, which has nothing to do with the PR.

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.

3 participants