f32/f64 to Ratio conversion

[sdroege]

Is there any interest for having conversion from f32/f64 to Ratio? I've implemented that in my code, based on a continued fractions algorithm: https://github.com/sdroege/gst-plugin-rs/blob/d72f1f716b630bc31973fc559c5f5d82255965a1/gst-plugin/src/utils.rs#L53

The same algorithm is used in GStreamer since a decade or so, for handling framerates stored as a floating point number and similar things, and it generally gives good results, but it's of course not the only way of doing that and depending on the input values another algorithm might find "nicer" results.

If there's interest, I could prepare a pull request for moving that code over (and generalize it a bit).

[cuviper]

There's already from_float on Ratio<BigInt> (aka BigRational), which is a bit easier since it can represent the full mantissa and exponent precisely.

I think it would be very reasonable to add the same method for Rational32 and Rational64. Even better if we could do it for generic Ratio<T>, but that seems harder to deal with generic boundary conditions, and we'd also want specialization to let the BigRational still use a direct conversion.

[cuviper]

Hmm, there might be a downside (breaking change?) for type inference though, since right now you can write just Ratio::from_float and infer that it will be a BigRational.

[sdroege]

We could make it a different function. ::approximate_from_float() or similar, as this will just give an approximation (should the error be returned?) and not an exact value like ::from_float() currently does.

[cuviper]

How about implementing FromPrimitive? That would get us from_f32, from_f64, and all the integer types too.

[sdroege]

Seems like an option, yes. Has the disadvantage that it would be for primitive types only, and not for Float. Not sure if that matters :)

[cuviper]

Fair point! I think FromPrimitive is probably good enough, worthwhile on its own, and still leaves the hypothetical door open for something even more general.

[sdroege]

Let's go with that then :) thanks

[sdroege]

With some refactoring I got it generalized to

fn float_to_rational<T, F>(val: F, max_error: F, max_iterations: usize) -> Option<Ratio<T>>
    where T: Integer + Signed + Bounded + NumCast + Copy,
          F: Float + NumCast;

I could use that in FromPrimitive, but a separate function implemented for the above on Ratio would also seem useful.

[sdroege]

... which opens the question for what Ratio FromPrimitive should be implemented. In case of BigInt you would like to do the direct translations, in case of Integer + Signed + Bounded + NumCast + Copy you would like to do the float approximation. Both at once is not possible due to possible overlaps.

It could be implemented for Ratio to Ratio (or i128), but that seems limiting.

[sdroege]

What I would propose is something like the following for now, until a good plan for FromPrimitive appears :)

impl<T> Ratio<T>
    where T: Integer + Signed + Bounded + NumCast + Copy
{
    pub fn approximate_from_float<F: Float + NumCast>(val: F, max_error: F, max_iterations: usize) -> Option<Ratio<T>> {
        [...]
    }
}

impl<T> Ratio<T>
    where T: Integer + Bounded + NumCast + Copy
{
    pub fn approximate_from_float_unsigned<F: Float + NumCast>(val: F, max_error: F, max_iterations: usize) -> Option<Ratio<T>> {
        [...]
    }
}

max_error and max_iterations could also be hardcoded if you think that makes more sense.

[cuviper]

Both at once is not possible due to possible overlaps.

Someday, we'd probably have a default impl for the approximation, then specialized for BigInt, but that's not possible today. The alternative is to implement the approximation explicitly for each primitive int (probably by macro), and do BigInt separately. We do a fair amount like that already. That misses out on third-party integer types, but having something is better than nothing.

We also don't have to choose one or the other. We can have both FromPrimitive and your methods, with one building on the other. Bikeshedding a little, I think I'd call it just approximate_float, and I'm not sure we really need an unsigned version.

max_error and max_iterations could also be hardcoded if you think that makes more sense.

If it's possible to hardcode reasonable choices, especially considering the different ranges of T, then I think that's better. I'd rather not specify/expose details of what algorithm we're using, nor expect users to understand how these variables should be used.

[sdroege]

Once specialization is there, this should also be possible to handle with a single function. Or would that only apply to traits? Here we would have the version with the Signed constraint as strictly more specific.

As such it might make sense to keep the unsigned version out for now and later have both there, one being a specialization of the other.

[sdroege]

OK, I'll prepare an initial pull request to add these for all fundamental integer types then, generated via macros and calling the generalized version. With hardcoded values for these.

At a later time this can be improved then, but I agree that having something would be better than nothing as a first step

[cuviper]

I believe this was completed in #285.