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

Inexact results from str::float::from_str #7648

Closed
SimonSapin opened this issue Jul 8, 2013 · 12 comments
Closed

Inexact results from str::float::from_str #7648

SimonSapin opened this issue Jul 8, 2013 · 12 comments
Labels
P-medium Medium priority

Comments

@SimonSapin
Copy link
Contributor

Some inputs to std::float::from_str give a value that prints as an integer, but does not compare equal to it:

rusti> std::float::from_str("0.67e3").get()
670
rusti> std::float::from_str("0.67e3").get() == 670f
false
rusti> std::float::to_str_hex(670f)
~"29e"
rusti> std::float::to_str_hex(std::float::from_str("0.67e3").get())
~"29e.00000000002"

For comparison, Python does not have this issue:

>>> float('.67e3') == 670
True
@SimonSapin
Copy link
Contributor Author

As jensnockert points out on IRC, 0.67e3 should parse to an integer float, so 29e.00000000002 is definitely wrong.

@auroranockert
Copy link
Contributor

It should be exactly equal, since 670.0 is exactly representable in IEEE 754. #7030 is probably the same issue.

Ps. Even with a float RHS, Python gives the correct answer

>>> float('.67e3') == 670.0
True

@krdln
Copy link
Contributor

krdln commented Jul 8, 2013

This line is responsible, I think.
https://github.com/mozilla/rust/blob/master/src/libstd/num/strconv.rs#L588
Maybe calculation of exponential part should be before fraction calculation?

@auroranockert
Copy link
Contributor

No idea, parsing floats is non-trivial. considering Java, PHP &c. have had severe DOS issues related to parsing floats, I think we should just borrow known-good code.

@Kimundi
Copy link
Member

Kimundi commented Jul 8, 2013

Yeah the current implementation just uses an very naive approach for float parsing and emitting.

The problem is that doing it proper is both hard and slow. To get an optimum result for parsing/printing a float from/to a base-n string representation requires bignum fractionals. (where optimum == "shortest unambigious string representation in base n" in a roundtrip float -> string -> float)

There exist algorithms specifically tailored to base 10 that can get around that by only requiring bignum ints, or even regular fixed sized integers, and working with bases 2, 4, 8, 16 and 32 is trivial, so I guess we'd ultimately want to incorporate all those techniqes by having different code paths to take, depending on input and base.

@pnkfelix
Copy link
Member

visiting for bug triage, email from 2013-08-26.

It would be nice to fix this. I haven't looked at the rust implementation in strconv.rs yet. But I believe that Will Clinger's paper from PLDI 1990 is a pretty common reference for this, see

And yes, I think these algorithms are specially tailored to base 10, as Kimundi noted.

@ghost ghost assigned pnkfelix Aug 29, 2013
@emberian
Copy link
Member

emberian commented Jan 1, 2014

Triage. This is still a problem. The papers pnkfelix links look really nice.

@huonw
Copy link
Member

huonw commented Jan 1, 2014

cc #6220

@pnkfelix
Copy link
Member

This needs to be properly triaged.

Nominating (I suspect it will be flagged as merely P-high, as that was what was done with #7030).

@pnkfelix
Copy link
Member

pnkfelix commented Nov 6, 2014

Assigning P-high, not 1.0.

@pnkfelix pnkfelix added P-medium Medium priority and removed I-nominated labels Nov 6, 2014
@IvanUkhov
Copy link
Contributor

Hello,

From a discussion on the #rust IRC channel today:

fn main() {
    let a = from_str::<f32>("3.141592").unwrap();
    let b = 3.141592f32;
    println!("{}", a == b); // false
}

So, the implementations of string parsing in the compiler and in from_str differ. It seems many people find this inconsistency rather confusing.

Regards,
Ivan

@huonw
Copy link
Member

huonw commented May 31, 2015

Closing as a forward-dupe of #24557, since that issue has more info.

@huonw huonw closed this as completed May 31, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P-medium Medium priority
Projects
None yet
Development

No branches or pull requests

8 participants