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

Avoid allocation when computing norm of MVector #1131

Merged
merged 2 commits into from
Feb 18, 2023

Conversation

mateuszbaran
Copy link
Collaborator

This is one way to fix #1126, another would be to convert any static array to SArray before the norm is calculated. I don't have a strong preference one way or another.

@ufechner7
Copy link

Any idea why the tests with Julia nightly fail?

@mateuszbaran
Copy link
Collaborator Author

The failure on Julia nightly is actually quite amusing. A test was marked as @test_broken, intended to indicate that an expression result type is not inferred. At some point the issue was fixed, but the test wasn't failing (as would be expected when @test_broken succeeds) because the expression returned an SVector instead of a Bool, which is incorrect. So the test was always accepting it as broken for the wrong reason. Julia decided to change @test_broken to interpret not returning a Bool as a failure, so it broke the incorrect @test_broken we have in StaticArrays.jl.

@ufechner7
Copy link

Thanks for the detailed explanation!
Now it only needs to get merged...

@mateuszbaran mateuszbaran merged commit d1e595a into master Feb 18, 2023
@mateuszbaran mateuszbaran deleted the mbaran/fix-norm-allocation branch February 18, 2023 18:21
@ufechner7
Copy link

Thanks for merging!
I tested #master, and it works.
On the other hand, on my Laptop I see a 12% performance penalty compared with the older version of StaticArrays. I will re-test on my Desktop which should give more reliable results.

@mateuszbaran
Copy link
Collaborator Author

You're welcome 🙂 .

The new norm is slightly slower but on the other hand it is more accurate (see discussion in #975 ).

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.

StaticArrays 1.5.13 breaks unit tests in other packages.
3 participants