Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #961 +/- ##
==========================================
- Coverage 98.21% 98.21% -0.01%
==========================================
Files 135 135
Lines 8002 7995 -7
==========================================
- Hits 7859 7852 -7
Misses 143 143
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
This is OK but you should at least carry over the |
Technici4n
left a comment
There was a problem hiding this comment.
I guess this is fine if you just want to get this out of the door!
|
What do you mean by "never really made sense to do that"? |
|
Perhaps we should just merge the Mooncake PR and do everything at once |
|
What I mean is that |
|
looks good, thanks! |
|
Let's just get https://github.com/chalk-lab/Mooncake.jl/pull/980/changes merged and we'll clean up the DI bindings as we bump our compat bound to Mooncake v0.5.1 |
|
So I tracked down the reason behind the existence of Initially, this used the handrolled function copyto!!(dst::Number, src::Number) = convert(typeof(dst), src)
copyto!!(dst, src) = copyto!(dst, src)But then, in #809, I replaced this function with the one from Mooncake, neglecting the fact that it doesn't accept inputs of different (el)types. Which means the functionality has been essentially useless for a while and we should remove it during the switch to Mooncake 0.5, now that friendly tangents take care of conversion. |
|
I see, that makes sense, thanks for investigating. ^^ |
|
Tests currently fail in forward mode with friendly tangents and in-place function because the f!_with_return(y, x) = begin
f!(y, x)
return y
end |
|
Thanks for the work! |
Configas a whole instead of parsing it_righttypecachesSupersedes #960
See also chalk-lab/Mooncake.jl#979
Note that this still uses internal conversion machinery from Mooncake