Add HyperDualNumbersExt #179
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #179 +/- ##
==========================================
+ Coverage 86.42% 90.40% +3.97%
==========================================
Files 13 14 +1
Lines 1002 1146 +144
==========================================
+ Hits 866 1036 +170
+ Misses 136 110 -26
☔ View full report in Codecov by Sentry. |
|
Mind adding tests, adding hyper duals as a test dependency? |
test/hyperduals.jl
Outdated
|
|
||
| Octavian.matmul!(C1, A1, B1, α, β) | ||
| LinearAlgebra.mul!(C2, A2, B2, α, β) | ||
| @test C1 ≈ C2 |
There was a problem hiding this comment.
| @test C1 ≈ C2 | |
| @test reinterpretH(C1) ≈ reinterpretHD(C2) |
|
|
||
| function reinterpretHD(T, A) | ||
| tmp = reinterpret(T, A) | ||
| return tmp[1:4:end, :] |
There was a problem hiding this comment.
reinterpreting a
This is essentially checking only the val part of the matrix
I will do some updates and name the tests properly on what they are checking.
There was a problem hiding this comment.
Don't we also want to check the epsilons?
There was a problem hiding this comment.
I thought what you mean in #179 (comment) is that we only check val part. Sorry for misunderstanding that and I will fix it now.
There was a problem hiding this comment.
I meant that is what isapprox only checks the real part, so we need to reinterpret to check the entire thing.
There was a problem hiding this comment.
I now check the entire thing and it should be ready for review. Sorry for iterating so many times.
|
Stucked on a bug on two dual arrays since I was not notcing it would be a bit different as in
Now I check also the correectness of the epsilons and the |
chriselrod
left a comment
chriselrod
left a comment
There was a problem hiding this comment.
Okay, great!
One last thing: mind bumping the version, so we can tag a new release?
| A1dual = randdual(A1) | ||
| C1dual = randdual(C1) | ||
|
|
||
| A2dual = deepcopy(A1dual) |
There was a problem hiding this comment.
Not a big deal, but some of these copies are unnecessary.
Just bumped to 0.3.23. Thank you for your review. |
2 participants
Ref #178 (comment)
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.