Conversation
add QR gauge projection
Codecov Report❌ Patch coverage is
🚀 New features to boost your workflow:
|
|
|
||
| # compute pullbacks | ||
| $f_pullback!(dA, Ac, DVc, dDVtrunc, ind) | ||
| $f_pullback!(dA, Ac, DV, dDVtrunc, ind) |
There was a problem hiding this comment.
How did this escape previous testing?
There was a problem hiding this comment.
I am not entirely sure, but it seems like the test_pullback_match functions had some holes in the testing functionality
| # compute pullbacks | ||
| svd_pullback!(dA, Ac, USVᴴc, dUSVᴴtrunc, ind) | ||
| zero!.(dUSVᴴtrunc) # since this is allocated in this function this is probably not required | ||
| svd_pullback!(dA, Ac, USVᴴ, dUSVᴴtrunc, ind) |
There was a problem hiding this comment.
Same question here; was this not tested before? This could not have worked.
There was a problem hiding this comment.
Same reply here, because the in-place functions weren't really tested very well, this slipped through the cracks
Co-authored-by: Jutho <Jutho@users.noreply.github.com>
|
I guess I really don't see the point of removing all the |
|
@kshyatt I think all your comments are now addressed, although I think this did make me have to touch a bit more of the files 🙃 I have some remaining questions with the rank-deficient QR/LQ tests, since it seems like I don't actually have to "gauge-fix" the |
3 participants
This is a somewhat large refactor of the Mooncake test suite that hinges on the idea to wrap the in-place functions in such a way that they become admissible to the internal Mooncake testing framework. (Based on ideas suggested by @Jutho, thanks!)
The basic idea is that for a function
f!(A, input, alg)we need to:Aspace to not count finite-differences resultsinputvariablesI also somewhat reorganized the tests to get a slightly better overview of the mooncake tests.
Finally, this also allowed me to find some small mistakes in the mutating rules, which I have here fixed.
This is also relevant for #174 and #173.