add specializations svd_trunc(!) for TruncatedAlgorithm#171
Conversation
Codecov Report❌ Patch coverage is
🚀 New features to boost your workflow:
|
| # compute pullbacks | ||
| $f_pullback!(dA, Ac, DVc, dDVtrunc, ind) | ||
| zero!.(dDVtrunc) # since this is allocated in this function this is probably not required | ||
|
|
||
| # restore state | ||
| copy!(A, Ac) | ||
| copy!.(DV, DVc) |
There was a problem hiding this comment.
To be consistent with the other adjoints, the copy!(A, Ac) would happen before calling the pullback, and then you can just pass A in the f_pullback! call. I am not opposed to the current order/approach but do value consistency as it helps in reading the code at a later stage.
There was a problem hiding this comment.
I think I would like to do a follow up PR where I refactor a bunch of this into the same order and format, but didn't want to pollute this PR too much with simple formatting changes.
| # compute pullbacks | ||
| $f_pullback!(dA, Ac, DVc, dDVtrunc, ind) | ||
| zero!.(dDVtrunc) # since this is allocated in this function this is probably not required | ||
|
|
||
| # restore state | ||
| copy!(A, Ac) | ||
| copy!.(DV, DVc) |
There was a problem hiding this comment.
Same comment about order of operations.
|
I think I addressed most of the remarks here, except for the order of operations. Would you be okay with me leaving that for a follow-up PR? I would like to refactor some of this code a bit more, and I don't want to pollute this PR with a bunch of formatting changes if possible. |
3 participants
No description provided.