elixir-nx / nx

Multi-dimensional arrays (tensors) and numerical definitions for Elixir
2.65k stars 192 forks source link

Creating tensor from float decimal issue #448

Open torepettersen opened 3 years ago

torepettersen commented 3 years ago

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
>
josevalim commented 3 years ago

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.

polvalente commented 3 years ago

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:

josevalim commented 3 years ago

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

DianaOlympos commented 3 years ago

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

samuelmanzanera commented 1 year ago

Hello. Is this issue still opened?

I also got an unexpected behavior:

iex> Nx.tensor(996372700.0)
#Nx.Tensor<
  f32
  996372672.0
>
polvalente commented 1 year ago

@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

samuelmanzanera commented 1 year ago

Ok thank you. I will try definitely.

DianaOlympos commented 1 year ago

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

josevalim commented 1 year ago

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

DianaOlympos commented 1 year ago

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

josevalim commented 1 year ago

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

DianaOlympos commented 1 year ago

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 commented 1 year ago

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