Changed isone type to AbstractMatrix + added test #41634
Conversation
|
Thanks @ThatcherT, and welcome to the Julia community. Could you please add another test that returns |
|
Thanks for the feedback! I will implement. @dkarrasch |
|
@dkarrasch I added a positive test for a non-strided array. |
test/numbers.jl
Outdated
| @test !isone(zeros(Int, 5, 5)) | ||
| @test isone(Matrix(1I, 5, 5)) | ||
| @test !isone(view(rand(5,5), [1,3,4], :)) | ||
| @test isone(view(Diagonal([1,1, 1]), [1,2], 1:2)) |
There was a problem hiding this comment.
It appears that this was working before, but it was doing effectively x == one(x), which created a whole dense copy in memory. So the new feature with this PR is that in most cases (except for things like AbstractQ from the qrs) the isone-check is now allocation-free. So we should actually test that as well:
| @test isone(view(Diagonal([1,1, 1]), [1,2], 1:2)) | |
| Dv = view(Diagonal([1,1, 1]), [1,2], 1:2) | |
| @test isone(Dv) | |
| @test (@allocated isone(Dv)) == 0 |
The last test fails currently.
|
I've implemented the changes you suggested, including asserting |
|
Sorry for the delay. Thank you for your contribution! |
This is my first contribution! I've made an effort to follow the instructions given in CONTRIBUTING.md. I added a test to show what was breaking before this fix.
Fixes #25516.