Add NaNMath extension for Unitful quantities#824
Add NaNMath extension for Unitful quantities#824sostock merged 5 commits intoJuliaPhysics:masterfrom
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #824 +/- ##
=========================================
Coverage ? 90.00%
=========================================
Files ? 22
Lines ? 1701
Branches ? 0
=========================================
Hits ? 1531
Misses ? 170
Partials ? 0 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
I wonder whether it makes sense to define these functions on dimensions and units as well, not just quantities? Of course, they are not different from the regular |
Co-authored-by: Sebastian Stock <42280794+sostock@users.noreply.github.com>
That seems unnecessary to me, I think, although I am open to discussion on that point. The current state of affairs in NaNMath is that NaNMath.sqrt(u"m") === sqrt(u"m")holds. I think the only reason to add more dispatches here would just be as a safeguard in case that fallback gets removed from NaNMath, and even for that, I would be a little surprised to see someone calling |
…mensions, will show up here
|
That said, it's easy enough to add a test that ensures we will know if |
|
Ah, I didn’t realize that NaNMath has generic fallback methods. This looks good to me then. |
Co-authored-by: Sebastian Stock <42280794+sostock@users.noreply.github.com>
The current version of NaNMath.jl doesn't work on Unitful quantities; e.g.
should return
NaN*u"m"but currently errors. (The point of NaNMath is that it should return NaN rather than error, which is helpful when e.g. adaptive solvers get outside of feasible space.)This was discussed on the NaNMath side at JuliaMath/NaNMath.jl#86 but that seems to have stalled out. The extension to Unitful of the
sqrtandpowmethods is easy enough; NaNMath has other functions (e.g. for functions likesinthat take dimensionless input, or aggregators likesum) but I think those are not as necessary.