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

[SR-106] description returns a rounded value of Double #42728

Closed
swift-ci opened this issue Dec 7, 2015 · 10 comments
Closed

[SR-106] description returns a rounded value of Double #42728

swift-ci opened this issue Dec 7, 2015 · 10 comments
Assignees
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. CustomStringConvertible Area → standard library: The `CustomStringConvertible` protocol Double Area → standard library: The `Double` type standard library Area: Standard library umbrella unexpected behavior Bug: Unexpected behavior or incorrect output

Comments

@swift-ci
Copy link
Collaborator

swift-ci commented Dec 7, 2015

Previous ID SR-106
Radar rdar://problem/40565639
Original Reporter wczekalski (JIRA User)
Type Bug
Status Resolved
Resolution Done

Attachment: Download

Environment
>lldb -version
lldb-340.4.110.1
Additional Detail from JIRA
Votes 0
Component/s Standard Library
Labels Bug
Assignee @tbkka
Priority Medium

md5: 295299b47bb427386aa275afabcc8598

is duplicated by:

relates to:

Issue Description:

It looks like the string representation of a Double returned by description is less precise than the Double itself. Look at the example below.

let point = CGPoint(x: 350.00000000000017, y: 0)
let value: CGFloat = 350.0

if point.x == value { // correctly, will never be reached
  print("")
}
print(point.x) // prints 350.0
@swift-ci
Copy link
Collaborator Author

swift-ci commented Dec 7, 2015

Comment by Aaron Brager (JIRA)

I'm just poking around in the Swift code base for the first time so I could be way off base here.

I think the LLDB output uses the CustomStringConvertible protocol to display the text.

./stdlib/public/SDK/CoreGraphics/CGFloat.swift.gyb implements it like so:

@_transparent extension CGFloat : CustomStringConvertible {
  /// A textual representation of `self`.
  public var description: String {
    return native.description
  }
}

I'm not sure what native is, but I'm guessing it's a pointer to an NSNumber object wrapping the CGFloat. (Otherwise it wouldn't be able to respond to description.)

Indeed, only 350 displays if the rounded number is wrapped in NSNumber, even in Objective-C:

(lldb) p point.x
(CGFloat) $0 = 350.00000000000017
(lldb) p @(point.x)
(__NSCFNumber *) $1 = 0x00007fc7d04baa50 (double)350

So I think this may be an NSNumber issue, not a Swift issue.

@swift-ci
Copy link
Collaborator Author

swift-ci commented Dec 7, 2015

Comment by Wojtek Czekalski (JIRA)

aaron (JIRA User) it's a good point. However, .native is of Swift.Double type.

(lldb) po any_CGFloat.native.dynamicType
Swift.Double

@swift-ci
Copy link
Collaborator Author

swift-ci commented Dec 7, 2015

Comment by Wojtek Czekalski (JIRA)

aaron (JIRA User) I tried to investigate it a bit but I didn't manage to find `Double` definition along with `description` implementation :/

@swift-ci
Copy link
Collaborator Author

swift-ci commented Dec 7, 2015

Comment by Aaron Brager (JIRA)

Ah, so is this implementation the relevant one?

extension ${Self} : CustomStringConvertible {
  /// A textual representation of `self`.
  public var description: String {
    return _float${bits}ToString(self)
  }
}

This is from ./stdlib/public/core/FloatingPoint.swift.gyb. I think that constructs a method name, which is then implemented in ./stdlib/public/stubs/Stubs.cpp. Double is 64 bits so this code would be relevant:

extern "C" uint64_t swift_float64ToString(char *Buffer, size_t BufferLength,
                                          double Value) {
  return swift_floatingPointToString<double>(Buffer, BufferLength, Value,
                                             "%0.*g");
}

I don't know C++ very well but it looks like this swift_floatingPointToString passes off to swift_snprintf_l, which passes off to std::vsnprintf. I wonder if this is a floating point rounding issue (ie, the storage used by std::vsnprintf might be less precise than the number itself?) Again, just guessing.

@swift-ci
Copy link
Collaborator Author

swift-ci commented Dec 7, 2015

Comment by Wojtek Czekalski (JIRA)

I was clearly wrong with the label. print shows the wrong value, too. It's not a LLDB specific issue. I will change the description.

@swift-ci
Copy link
Collaborator Author

swift-ci commented Dec 7, 2015

Comment by Wojtek Czekalski (JIRA)

aaron (JIRA User) yup, we found the issue!

I put a breakpoint on swift_float64ToString. This is the stack trace (part of it)

#&#8203;0  0x000000010f860c10 in swift_float64ToString ()
#&#8203;1  0x000000010faee7d5 in Double.description.getter ()

When we look at the signature:

extern "C" uint64_t swift_float64ToString(char *Buffer, size_t BufferLength,
                                          double Value)

So, the second argument is BufferLength. I printed its value in the debugger.

(lldb) p $arg2
(unsigned long) $0 = 32

And boom!

This is the issue (github link)

@swift-ci
Copy link
Collaborator Author

swift-ci commented Dec 8, 2015

Comment by Wojtek Czekalski (JIRA)

#348
I'm not sure how the process looks like so I pasted a link to the PR here.

@tbkka
Copy link
Contributor

tbkka commented Mar 8, 2018

Useful background information: In #43071, Kevin Ballard provided links to some of the relevant algorithms, including Florian Loitsch' Grisu family of algorithms which provide good performance and accurate results. In particular, Grisu2 is simple, fast, and provides the optimal results 99% of the time. A recent paper by Andrysco, Jhala, and Lerner analyzes the shortcomings of Loitsch' Grisu2 and proposes a way to address them. Their Errol algorithm shows that Grisu-style algorithms can provide completely optimal formatting with very high speed.

For the last year, I've been experimenting with another Grisu2 variant that uses a lot of the same ideas in a slightly different way. I'd like to replace the description and debugDescription implementations for Float, Double, and Float80 with this new implementation.

This would provide the following:

  • Round-trip accuracy. For any Double value d, it guarantees that Double(d.description) == d exactly with no rounding error. (See [SR-491] Double#description is not roundtrip-safe #43108)

  • Short. The printed form uses the fewest possible digits. (Loitsch's original Grisu2 only finds the shortest form 99% of the time.)

  • Close. When there is more than one accurate, short form, this selects the one that is closest to the original number.

  • Speed. The underlying formatter is more than twice as fast as Grisu3 or Errol4 and between 3x and 30x faster than the Dragon4 algorithm used in most standard C libraries. (Dragon4 takes a different amount of time depending on the exponent of the value; Grisu-style algorithms can run in constant time.)

Of course, when used by the description property, some of the performance gain is masked by the overhead of creating a new string. In subsequent work, I'd also like to expose new APIs that provide some support for formatting floating-point numbers without always creating a new string. But I think we can drop in the new formatter behind the existing description and debugDescription properties to address some of the most obvious issues first.

@tbkka
Copy link
Contributor

tbkka commented Mar 24, 2018

Please see #15474

@stephentyrone
Copy link
Member

Done with commit 97a934c.

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
@AnthonyLatsis AnthonyLatsis added Double Area → standard library: The `Double` type unexpected behavior Bug: Unexpected behavior or incorrect output labels May 26, 2023
@AnthonyLatsis AnthonyLatsis added the CustomStringConvertible Area → standard library: The `CustomStringConvertible` protocol label May 26, 2023
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. CustomStringConvertible Area → standard library: The `CustomStringConvertible` protocol Double Area → standard library: The `Double` type standard library Area: Standard library umbrella unexpected behavior Bug: Unexpected behavior or incorrect output
Projects
None yet
Development

No branches or pull requests

4 participants