-
Notifications
You must be signed in to change notification settings - Fork 5
Add tests for image and null space for GPU #82
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
Conversation
|
Your PR no longer requires formatting changes. Thank you for your contribution! |
|
This will need #80 to merge first |
lkdvos
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm happy to merge this and then deal with the possible orth changes, or the other way around. I'm not sure what is easier?
|
I'd say merge this once I get the AMDGPU tests passing, then I can do another PR post your changes? |
|
@lkdvos I had to modify the |
Codecov Report✅ All modified and coverable lines are covered by tests.
... and 1 file with indirect coverage changes 🚀 New features to boost your workflow:
|
lkdvos
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good overall to me. I wasn't sure if we should first merge this or first the ortho rework, but it looks like that might still require some additional time/reviews so LGTM first
| U, S = US | ||
| extended_S = vcat(diagview(S), zeros(eltype(S), max(0, size(S, 1) - size(S, 2)))) | ||
| ind = MatrixAlgebraKit.findtruncated(extended_S, strategy) | ||
| trunc_cols = collect(1:size(U, 2))[ind] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| trunc_cols = collect(1:size(U, 2))[ind] | |
| trunc_cols = ind isa AbstractVector{Integer} ? ind : axes(U, 2)[ind] |
Does this happen to work as well?
lkdvos
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left one last comment more out of curiosity than actual importance, otherwise good to go for me!
* Bump v0.6 * rename `gaugefix` -> `fixgauge` * reduce unnecessary warnings * fix `copy_input` signatures in Mooncake tests * Add changelog to docs
No description provided.