Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

What does it mean that a value cannot be represented by Ratio<T>? #79

Closed
Enyium opened this issue Jan 27, 2023 · 2 comments
Closed

What does it mean that a value cannot be represented by Ratio<T>? #79

Enyium opened this issue Jan 27, 2023 · 2 comments

Comments

@Enyium
Copy link

Enyium commented Jan 27, 2023

I use this crate to convert an f64 into an fps value (video framerate) consisting of u32 numerator and denominator. The documentation for Ratio::<u32>::from_f64() reads:

Converts a f64 to return an optional value of this type. If the value cannot be represented by this type, then None is returned.

I did a few tests and transformed your explanation into the following doc comment for my Fps::from_f64() function:

Panics if, irrationally for a frame rate, the value is so large that a u32 numerator can't describe it.

Is that accurate and does it cover everything?

My observations (using pi, shifting the decimal point):

  • A smaller and smaller value kills precision after a certain point. E.g., 0.0000000000314159 gives me numerator: 0, denominator: 1.
  • More and more digits before the decimal point give you a smaller and smaller denominator until it reaches 1. After that, you get None.

I wondered whether there can be a solution (a fraction) that's so imprecise (not accurately covering enough f64 digits) that "the value cannot be represented by" the Ratio<u32>?


If my description concerning the numerator only is more accurate, maybe your doc comment should also describe it that way to not leave questions open.

@dnsl48
Copy link
Owner

dnsl48 commented Jan 27, 2023

Hi @Enyium,
the FromPrimitive trait is re-imported from the num_traits crate. The documentation piece you are referring to is sourced from there.
Ratio type is also re-imported from the num_rational crate.
This crate provides GenericFraction, which implements From<f64>.

I did a few tests and transformed your explanation into the following doc comment for my Fps::from_f64() function:

sorry, I am not sure what you mean exactly, but ideally the method shouldn't panic. Semantically the method should return None if cannot convert into the relevant type.

@Enyium
Copy link
Author

Enyium commented Jan 27, 2023

Ratio type is also re-imported from the num_rational crate. This crate provides GenericFraction, which implements From<f64>.

Thanks for the referral. Naturally I must use this crate then. I'm settling the matter with its authors.

sorry, I am not sure what you mean exactly

I'd like to specify in my API docs when exactly the method Fps::from_f64() can fail, so a lib user knows how things stand. But that depends on when my deps can fail.

ideally the method shouldn't panic. Semantically the method should return None if cannot convert into the relevant type.

To cite a Reddit comment:

if your library is the source of a panic, then one of the following should be true:

  • Your library has a bug.
  • Your library documents a precondition of a public API item that, when not met, causes a panic. Therefore, the user of your library has misused your library, and their code has a bug.

My thought was: Frame rates already become nonsensical after values of 200. A lib user could cap the input, from whereever it's sourced, to, e.g., 100_000 or something lower. A frame rate of 0xffff_ffff would be insane. The high-speed camera record lies at 1_160_000 fps, which is still well below 0xffff_ffff. But high-speed camera videos are always played back at much lower frame rates, and will also be stored in video files with sensible frame rates.

But since an f64 can be invalid in more ways than just being extraordinarily large, I proceeded to return an Option<...>.

@Enyium Enyium closed this as completed Jan 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants