Implementing epsilon function to retrieve EPSILON constant #231
Conversation
|
It's a breaking change to add new methods to a trait without a default
implementation.
Note the f32/f64 values are just 2^-23 and 2^-52, directly related to the
mantissa size. We don't know the mantissa for generic floats either, but
perhaps there's some other way to estimate epsilon. It doesn't have to be
fast or pretty, just best effort to avoid outright breaking.
(Or we can wait for the mass breaking change 0.2 that we keep talking
about...)
|
|
Thank you for your comments! I didn't realize this was a breaking change - sorry about that! I'm assuming that to avoid breakage you are implying we could provide a default implementation? I'm not too sure how this would work (without bringing in FromPrimitive or something - which is also a breaking change?). If you have any suggestions I'm happy to try and implement them or you can feel free to push directly to this branch. |
|
Right, it avoids the break if you have a default impl. Float doesn't have FromPrimitive, but it does have NumCast which can probably help. (I always thought that was a weirdly defined trait.) |
|
So perhaps we can provide a default implementation like this: fn epsilon() -> T {
T::from(f32::EPSILON)
}And we leave the overrides for f32 and f64 in the macro? This way we have the correct implementations for f32 and f64 with a sensible (enough?) default for users implementing |
|
Yeah, that's probably Good Enough -- it's hard to be confident that any other heuristic would truly be better. It's |
|
I've implemented the default as above. If you have a better panic message let me know! (I also added a panic header in the docs - I can remove it if you prefer) |
|
Looks good, thanks! When we do make breaking changes, we might drop this default impl to make it mandatory for implementors. @homu r=hauleth |
|
📌 Commit 381942e has been approved by |
Implementing epsilon function to retrieve EPSILON constant Hey! This PR exposes a new `epsilon` function in the `Float` trait so that users can access the `EPSILON` constant on the float types. I figured as this was such a minimal change it was easier to get a PR in offering the change then write up an issue. For me this is a valuable addition. When writing linear algebra or other optimization routines with generic floats we often want to check if some stopping criteria is reached, often something like: `(a - b).abs() < eps`. Having access to a standard _small value_ would make this a little easier.
|
⚡ Test exempted - status |
Great, thank you! Would you like me to open an issue to track the removal of the default impl with the next minor version bump? (I agree that it is probably best to remove it) |
|
Sure, a tracking issue would be fine. |
|
FWIW this broke |
|
That's unfortunate, but allowable as a minor change under RFC#1105, thanks to the default impl. It sure would be nice to have Crater access ourselves though. I'll file heads-up bugs on those packages. |
|
cc @brson for the crater thing. |
…n-to_value Serialize Map with integer keys
Hey!
This PR exposes a new
epsilonfunction in theFloattrait so that users can access theEPSILONconstant on the float types. I figured as this was such a minimal change it was easier to get a PR in offering the change then write up an issue.For me this is a valuable addition. When writing linear algebra or other optimization routines with generic floats we often want to check if some stopping criteria is reached, often something like:
(a - b).abs() < eps. Having access to a standard small value would make this a little easier.