Conversation
|
Your PR no longer requires formatting changes. Thank you for your contribution! |
|
This will need #80 to merge first |
lkdvos
left a comment
lkdvos
left a comment
There was a problem hiding this comment.
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
lkdvos
left a comment
There was a problem hiding this comment.
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.
| 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
lkdvos
left a comment
There was a problem hiding this comment.
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
2 participants
No description provided.