nix icon indicating copy to clipboard operation
nix copied to clipboard

Some floats are printed like ints

Open candeira opened this issue 6 years ago • 14 comments

I believe this is a very minor bug, but still a bug. Minor as it is, it also seems a good one for a first time user, so I'd like to give it a try.

Floats that could be coerced to ints are printed with no decimals, so the printed value looks like an int literal:

$ nix repl
Welcome to Nix version 2.2.2. Type :? for help.

nix-repl> builtins.typeOf (2 + 2.5)
"float"

nix-repl> builtins.typeOf (2 + 2.0)
"float"

nix-repl> 2 + 2.0
4

nix-repl> builtins.typeOf (2 + 2.0)
"float"

Here's the subsequent IRC channel talk, which explains rationale for dealing this issue:

12:53 < ivan> kandinski: I suspect this is a bug that could be fixed
12:54 < ivan> could be this in src/libexpr/eval.cc
12:54 < ivan>     case tFloat:
12:54 < ivan>         str << v.fpoint;
12:54 < kandinski> Yeah, I'm writing tutorial materials right now, that's how I came to it.
13:01 < __monty__> 4's a valid float literal though. I guess print then read isn't idempotent this way but there 
                   seems to be no coercion since typeOf correctly identifies it as a float?
13:02 < kandinski> yeah, it's just bad UX
13:02 < kandinski> in the sense that users can be confused of the type when they print it
13:09 < kandinski> I don't know a lot of C++, but it seems like it's C++ behaviour. NixFloat is a typedef'd double
13:10 < clever> kandinski: the problem would be the operator to apply << to whatever str is
13:10 < clever> with a double
13:10 < kandinski> clever, ah, so do you think << is overloaded there?
13:10 < kandinski> makes sense
13:10 < kandinski> everything else is standard C++ as far as I understand it
13:11 < clever> kandinski: the c++ stdlib should supply a << operator, for ostream and double, but does it do what 
                you expect?
          https://git.io/fjpnw
13:12 < kandinski> clever: right, so there are two questions. First one, what's the right behaviour? If the current 
                   one is deemed ok, then this is just a matter of documenting it and moving forward.
13:16 < kandinski> clever: Second question would be how to fix it if it's considered an issue that needs fixing.
13:17 < clever> kandinski: find your own function to convert a float to a string, then <<, that string
13:17 < __monty__> If there's a way to eval a string into nix then I'd prefer print then eval to be idempotent tbh.

I agree that idempotency of print/eval should be preferred, but the main reason is still reducing confusion to human users.

candeira avatar Sep 01 '19 13:09 candeira

Yeah that makes sense. This is just for the repl representation so changing it wouldn't have much of an impact.

nix-repl> toString (2 + 2.0)
"4.000000"

zimbatm avatar Sep 02 '19 09:09 zimbatm

We could also get rid of the ambiguity by dropping the integer datatype. So 4 would be a floating point number exactly equal to the integer 4.

edolstra avatar Sep 02 '19 11:09 edolstra

I marked this as stale due to inactivity. → More info

stale[bot] avatar Feb 18 '21 04:02 stale[bot]

I closed this issue due to inactivity. → More info

stale[bot] avatar Apr 28 '22 23:04 stale[bot]

Came across this in this new test https://github.com/NixOS/nix/pull/8566/files#diff-79ff5e5f6f39edd4a8a247ceb45dddde66f90d2c93ae89ebf37e16f69d0e8df4R160

Probably that has the same cause, and this may be an easy fix.

roberth avatar Jul 01 '23 18:07 roberth

We could also get rid of the ambiguity by dropping the integer datatype. So 4 would be a floating point number exactly equal to the integer 4.

This would be a breaking change considering

nix-repl> builtins.typeOf 1.0
"float"

nix-repl> builtins.typeOf 1  
"int"

Might have been worth changing in 2019, but floats have been around in the language for a long time now, so I don't think we should do this.

roberth avatar Jul 01 '23 18:07 roberth

I think this issue can be solved by improving Value::print, in a way that always outputs a decimal point when rendering a float. The method is not used in a way that affects the stability of evaluations, but rather:

  • CLI outputs and the repl
  • builtins.trace
  • nix-env manifest.nix. This would arguably be strictly an improvement to render floats as floats, but probably no floats are ever rendered there anyway.

So none of these will have an effect on instantiations.

roberth avatar Jul 01 '23 18:07 roberth

Triaged in Nix team meeting 2023-07-07:

Approved, as long as that doesn't change the language semantics

fricklerhandwerk avatar Jul 15 '23 17:07 fricklerhandwerk

This issue has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/2023-07-07-nix-team-meeting-minutes-69/30470/1

nixos-discourse avatar Jul 15 '23 17:07 nixos-discourse

I think this issue can be solved by improving Value::print, in a way that always outputs a decimal point when rendering a float. The method is not used in a way that affects the stability of evaluations, but rather [...]

This is not entirely the case. Structured attributes render a __json key in the environment section of the Aterm, and the value is the json representation. If you end up using floats there, it will cause different drv and output hashes.

Maybe it's not too late to ban floats inside structured attrs at least.

flokli avatar Jan 12 '24 11:01 flokli

structuredAttrs and toJSON do print a decimal point for all floats. This issue is about printing Nix values.

That's not to say JSON print of floats, or floats themselves aren't troublesome (e.g. https://github.com/NixOS/nix/issues/5733) but this particular issue, 3077, is easily solvable and will be closed when solved.

roberth avatar Jan 12 '24 12:01 roberth

Some insights as a new user, in a repl I did:

4/6    # got a path => surprised to say the least
4 / 6  # got 0 => ok, deterministic, strict int-only makes sense, no floats
4.0    # got 4, "do they treat these as integer version numbers?"

a search for nix floats points to:

  • the docs which don't have a single! example
    • at this point I'm questioning if the syntax has a suffix like "4.0f" to make float explicit
  • how to convert an int into a float which is done implicitly via addition!?
  • and this issue

so here I am questioning whether float support is experimental like flakes

I should also mention I went through the nix.dev tutorial and floats/floating point operations/conversions weren't mentioned either. I can't believe that not even floats are straight forward with nix, it's just so much unnecessary overhead to getting started

davidgtl avatar Nov 06 '24 21:11 davidgtl

@davidgtl thanks for pointing it out. Feel free to open PRs to fix taht, I do reviews somewhat regularly as long as it fits my time budget.

fricklerhandwerk avatar Nov 07 '24 04:11 fricklerhandwerk

I'm 99% sure this bug is caused by the following line in print.cc:

    void printFloat(Value & v)
    {
        if (options.ansiColors)
            output << ANSI_CYAN;
        output << v.fpoint();
        if (options.ansiColors)
            output << ANSI_NORMAL;
    }

The line output << vfpoint(); should do something like output << std::showpoint << v.fpoint() instead. Since showpoint mutates the stream object, to be absolutely belt-and-suspenders about this we should store and reset that formatting flag when we're done.

I'll open a PR for this sometime this week.

(showpoint docs)

Note: somewhat more concerning is the rounding that occurs on e.g. builtins.toString 4.6666663; this I believe is also caused by the use of streams, and we'd have to decide how to handle that. C++ naturally does not have--to my knowledge--an easy way of doing auto-precision.

Image

crertel avatar Dec 11 '25 06:12 crertel