Some numeric_limits and numeric helper functions were missing#331
Some numeric_limits and numeric helper functions were missing#331nholthaus merged 2 commits intonholthaus:masterfrom
Conversation
BenFrantzDale
left a comment
BenFrantzDale
left a comment
There was a problem hiding this comment.
I was hoping for this! I have to jump through hoops to get inf.
A few comments. Also, you should probably add tests in https://github.com/nholthaus/units/blob/master/unitTests/main.cpp
| static constexpr units::unit_t<Units, T, NonLinearScale> epsilon() | ||
| { | ||
| return units::unit_t<Units, T, NonLinearScale>(std::numeric_limits<T>::epsilon()); | ||
| } |
There was a problem hiding this comment.
Since this is epsilon near 1.0, I feel like it's not unit-invariant the same way lowest() is.
There was a problem hiding this comment.
I am not sure, it depends on the underlying type, or I don't understand something,
There was a problem hiding this comment.
My thought here is I can’t think of a case where you’d want epsilon that wasn’t better served by multiplying by std::numeric_limits<double>::epsilon(). Epsilon is the smallest increment you add to 1.0 to get a different number, but with units, 1.0 what? I can find the epsilon WRT 1.0_mm by multiplying by double-epsilon, but I can’t think of a use case in which the number isn’t magic. Does that make sense?
There was a problem hiding this comment.
Like if I change D between meters and millimeters then 1.0_mm + std::numeric_limits<D>::epsilon() would change truthiness, I think.
| return std::signbit(x()); | ||
| } | ||
|
|
||
| } |
There was a problem hiding this comment.
Should we provide https://en.cppreference.com/w/cpp/types/numeric_limits/is_specialized ?
There was a problem hiding this comment.
Yes, you are right, added with the new commit.
include/units.h
Outdated
There was a problem hiding this comment.
Consider adding denorm_min https://en.cppreference.com/w/cpp/types/numeric_limits/denorm_min
There was a problem hiding this comment.
Yes, that is missing too, included with the new commit
- added test for limits
Also added some unit tests for limits. Can you check them? Maybe you have some other tests in mind. |
|
great PR guys thanks!! |
3 participants
Extended std::numeric_limits class with some functions needed to use it with yaml-cpp.