Closed CheukHinHoJerry closed 1 year ago
Patch coverage: 86.95
% and project coverage change: +3.97
:tada:
Comparison is base (
00d50b3
) 86.42% compared to head (1a10ea4
) 90.40%.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.
Mind adding tests, adding hyper duals as a test dependency?
You can compare the answers with a naive or base multiplication.
I'd reinterpret to Float64
before comparing, because comparisons tend to only check the value, and not any of the partials.
Stucked on a bug on two dual arrays since I was not notcing it would be a bit different as in ForwardDiff.Dual
but just got that fix and it should be fine now. Basically I checked:
Now I check also the correectness of the epsilons and the reinterpredHD
function is now a sanity check which I use LinearAlgebra.mul!
to calcualte only the val
part of the result. I think this can be removed but it doesn't harm if we keep that.
Okay, great! One last thing: mind bumping the version, so we can tag a new release?
Just bumped to 0.3.23. Thank you for your review.
Ref https://github.com/JuliaLinearAlgebra/Octavian.jl/issues/178#issue-1758088520
Basically I just replace the type constrain of 'ForwardDiff.Dual' to 'HyperDualNumbers.Hyper' and create an extra module. Haven't done any serious test on it but it works okay on my end.
I guess it might be possible to combine into a single ext with ForwardDiff.Dual but I will just leave something workable for now since I am not sure where/how should I do this properly.