nx icon indicating copy to clipboard operation
nx copied to clipboard

Creating tensor from float decimal issue

Open torepettersen opened this issue 3 years ago • 13 comments

Creating a tensor from a float is causing a problem with the decimals:

Nx.tensor(0.43)
#Nx.Tensor<
  f32
  0.4300000071525574
>

The same goes if type is set to f16:

Nx.tensor(0.47, type: {:f, 16})
#Nx.Tensor<
  f16
  0.469970703125
>

But for f64 it seems to work:

Nx.tensor(0.47, type: {:f, 64})
#Nx.Tensor<
  f64
  0.47
>

torepettersen avatar Aug 31 '21 13:08 torepettersen

Oh, it is because they are in f32 and f16 and we are computing the shortest representation for f64. We would need to implement shortest representation algorithms for both f32 and f16.

josevalim avatar Aug 31 '21 14:08 josevalim

Thanks for the feedback! This is an issue with the precision of the float itself, not a problem with Nx.

https://www.h-schmidt.net/FloatConverter/IEEE754.html

Here you can check that the same representation for 0.43 is reached for 32-bit floats. I'm not sure if it allows for converting to f64 ou f16.

Also, if you're not familiar with floats, the following pure Elixir snippet might help shed some light into float rounding errors:

iex(1)> 0.1 + 0.2
0.30000000000000004
iex(2)> 7 / 100 * 100
7.000000000000001

edit: what @josevalim said is also relevant to the issue :sweat_smile:

polvalente avatar Aug 31 '21 14:08 polvalente

To be clear, the representation is correct, just not the shortest. /cc @dianaolympos

josevalim avatar Aug 31 '21 14:08 josevalim

Yeah i will take a look. Will take a bit of time, but we should be able to handle these

DianaOlympos avatar Aug 31 '21 14:08 DianaOlympos

Hello. Is this issue still opened?

I also got an unexpected behavior:

iex> Nx.tensor(996372700.0)
#Nx.Tensor<
  f32
  996372672.0
>

samuelmanzanera avatar Jan 25 '23 09:01 samuelmanzanera

@samuelmanzanera This is because Elixir represents floats as f64 by default, but Nx will infer floating-point values as f32 by default.

iex(1)> <<x::32-float-native, _::8>> = <<996372700.0::32-float-native, 0::8>>
<<195, 141, 109, 78, 0>>
iex(2)> x
996372672.0

If you pass type: :f64 as an option to Nx.tensor, it should return the input value properly:

iex(3)> Nx.tensor(996372700.0, type: :f64)
#Nx.Tensor<
  f64
  9.963727e8
>

Note that this isn't directly relevant to this issue, which aims to have a better short-form textual representation for floating point tensors

polvalente avatar Jan 25 '23 11:01 polvalente

Ok thank you. I will try definitely.

samuelmanzanera avatar Jan 25 '23 11:01 samuelmanzanera

For an update:

I had a look. Making this work for f32 and f16 is a lot of work on the test side, and I simply have not had the time to do that. At this point, this is too big for me to do in my limited free time rn.

:( sorry

DianaOlympos avatar Jan 25 '23 16:01 DianaOlympos

@DianaOlympos do you have any code to share? :)

josevalim avatar Jan 25 '23 17:01 josevalim

I dumped the current state here. The f64 works, it was done as a stepping stone. The f32 tests do not pass and partially this is because the tests are wrong. The tests are "stolen" and repurposed from the OTP one. That may have been a bad idea but there are so many edge cases I was not trusting myself to do it from scratch.

https://github.com/DianaOlympos/ken

DianaOlympos avatar Jan 25 '23 17:01 DianaOlympos

@DianaOlympos I wonder if we should just brute force it: for testing we write bindings to a C lib and we compare the outputs.

josevalim avatar Jan 25 '23 17:01 josevalim

That was on my todo list to explore but they use different string formats... But maybe if we test before making it a string...

And we just enumerate the 32 bits space, it is not that large

DianaOlympos avatar Jan 25 '23 17:01 DianaOlympos

That said i do not think there is a f16 reference implementation i would trust

DianaOlympos avatar Jan 25 '23 17:01 DianaOlympos