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

Float.toText only displays 6 fractional digits - we probably want a representation that would roundtrip as a literal. #88

Open
crusso opened this issue Jun 25, 2020 · 18 comments

Comments

@crusso
Copy link
Contributor

crusso commented Jun 25, 2020

see rts/float.c

@crusso
Copy link
Contributor Author

crusso commented Jun 25, 2020

Using specifier "%.16g" not "%f" yields better result, but I don't know if that's good enough.

rts/float.c

export as_ptr float_fmt(double a) {
  extern int snprintf(char *__restrict, size_t, const char *__restrict, ...);
  char buf[50]; // corresponds to 150 bits of pure floatness, room for 64 bits needed
  //  const int chars = snprintf(buf, sizeof buf, "%f", a);
  const int chars = snprintf(buf, sizeof buf, "%.17g", a);
  return text_of_ptr_size(buf, chars);
}

@crusso
Copy link
Contributor Author

crusso commented Jun 25, 2020

@rossberg any advice here?

@rossberg
Copy link
Contributor

rossberg commented Jun 26, 2020

Yes, %.17g is enough to roundtrip, I've used that in the Wasm interpreter, and it works for all the many corner cases in the test suite that exercise bit-precise rounding behaviour (at least OCaml's implementation of the formatting, which I assume is inherited from C).

However, in practice, users may not want to see umteen zeroes all the time. We'll have to add some richer formatting functions; toText should probably default to %f.

There's also %h, of course.

@ggreif
Copy link
Contributor

ggreif commented Jun 26, 2020

Something like toText(f : Float, prec : ?Nat8) would be easily realisable (in the compiler, for the interpreter probably hairier).

@rossberg
Copy link
Contributor

rossberg commented Jun 26, 2020

@ggreif, I think it should be a separate function from toText, though. But then we'd probably want s.th like SML's realfmt type, maybe extended with a hex case for %h.

In the interpreter that ought to be trivial (using OCaml's Printf).

@ggreif
Copy link
Contributor

ggreif commented Jun 26, 2020

@rossberg In OCaml we can't build the format string dynamically, can we?

val fmt : string = "%.9g"
# Printf.sprintf fmt 3.14159;;
Error: This expression has type string but an expression was expected of type
         ('a -> 'b, unit, string) format =
           ('a -> 'b, unit, string, string, string, string)
           CamlinternalFormatBasics.format6

@rossberg
Copy link
Contributor

rossberg commented Jun 26, 2020

Following SML's basis library, here is what I propose: a function Float.toFormattedText : (Float, Format) -> Text, where the type Float.Format is defined as follows:

type Format = {
  #fix : Nat;  // like "%.*f"
  #exp : Nat;  // like "%.*e"
  #gen : Nat;  // like "%.*g"
  #hex : Nat;  // like "%.*h" a.k.a. "%.*a"
  #exact;  // like "%.17g"
}

(Making precision non-optional, since that doesn't buy you much in this form of description.)

@rossberg
Copy link
Contributor

rossberg commented Jun 26, 2020

@ggreif, no, but a simple switch over the 4 or 5 formats is enough. The precision can be turned into a dynamic argument by using *.

@ggreif
Copy link
Contributor

ggreif commented Jun 26, 2020

That's what I mean by "hairier".

@rossberg
Copy link
Contributor

How is that "hairy"?

@rossberg rossberg transferred this issue from dfinity/motoko Jun 26, 2020
@ggreif
Copy link
Contributor

ggreif commented Jun 26, 2020

It has more hair :-)

@ggreif
Copy link
Contributor

ggreif commented Jun 26, 2020

But we'll still need some supporting primitives for this to fly, right? (I'll come up with a PR in dfinity/motoko soon.)

@rossberg
Copy link
Contributor

Perhaps just extend float_fmt with a mode and a precision arg?

@crusso
Copy link
Contributor Author

crusso commented Jun 26, 2020

Great, thanks guys. I'll leave that in your hands then @ggreif.

@ggreif
Copy link
Contributor

ggreif commented Jun 30, 2020

I totally misunderstood what "%.*f" means. I thought that the * is purely meta-notation. Another thing learned...

@rossberg
Copy link
Contributor

Now I see why you considered it hairy. :)

mergify bot pushed a commit to dfinity/motoko that referenced this issue Jul 1, 2020
The current `floatToText` primitive is (while being fine for `debug_show`) insufficient for several reasons:
- no round-trip
- not adjustable precision
- only decimal output.

dfinity/motoko-base#88 asks for better formatting capabilities. This PR implements the underlying primitive `floatToFormattedText `, while leaving the previous one as-is.

Introduces primitive `floatToFormattedText(num : Float, precision : Word8, mode : Word8)`
with `mode`:
  - (0) fixed format `"%.*f"`
  - (1) exponent format `"%.*e"`
  - (2) generic format `"%.*g"`
  - (3) hexadecimal format `"%.*h"`
@ggreif
Copy link
Contributor

ggreif commented Jul 1, 2020

Now I see why you considered it hairy. :)

Precisely.

@ggreif ggreif mentioned this issue Jul 1, 2020
2 tasks
@ghost ghost unassigned ggreif Jul 7, 2020
@ehoogerbeets
Copy link

Is there any way we can not have toFormattedText at all and make it very very clear in the documentation that a simple toText is not for displaying to users? Formatting floating point numbers for human consumption is a locale-dependent function, as there are many different ways that different cultures format numbers with varying thousands separators, digit groupings, decimal separators, rounding rules, etc. A separate number formatter class that takes a locale would be ideal and would keep floats and natural numbers clean and small.

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

4 participants