fixed numpy.prod failing tests

[ShreyanshBardia]

closes #20418
closes #20415
closes #13732
closes #8209

[github-actions]

Thanks for contributing to Ivy! 😊👏
Here are some of the important points from our Contributing Guidelines 📝:
1. Feel free to ignore the run_tests (1), run_tests (2), … jobs, and only look at the display_test_results job. 👀 It contains the following two sections:
- Combined Test Results: This shows the results of all the ivy tests that ran on the PR. ✔️
- New Failures Introduced: This lists the tests that are passing on main, but fail on the PR Fork. Please try to make sure that there are no such tests. 💪
2. The lint / Check formatting / check-formatting tests check for the formatting of your code. 📜 If it fails, please check the exact error message in the logs and fix the same. ⚠️🔧
3. Finally, the test-docstrings / run-docstring-tests check for the changes made in docstrings of the functions. This may be skipped, as well. 📚
Happy coding! 🎉👨‍💻

[Pchatain]

Hey @ShreyanshBardia thanks for working on this! It looks like in the CI tests, there are some failing tests for

test_numpy_ndarray_prod
test_numpy_prod

And I don't see any test results for sums_products_differences.test_numpy_prod. Do you know why these fail, and why that test doesn't appear to be running?

[ShreyanshBardia]

Hey @Pchatain, I'm not sure why this is happening, because when I ran the tests locally earlier the tests were passing and everything seemed to be working as expected.

[Pchatain]

Hey @ShreyanshBardia , to better understand, are all 3 of those tests passing on your end? When I checkout your PR and run pytest ivy_tests/test_ivy/test_frontends/test_numpy/test_ndarray/test_ndarray.py::test_numpy_ndarray_prod I get

=================================== short test summary info ====================================
FAILED ivy_tests/test_ivy/test_frontends/test_numpy/test_ndarray/test_ndarray.py::test_numpy_ndarray_prod[cpu-numpy-False-False] - AssertionError:  the results from backend numpy and ground truth framework numpy do not match
FAILED ivy_tests/test_ivy/test_frontends/test_numpy/test_ndarray/test_ndarray.py::test_numpy_ndarray_prod[cpu-jax-False-False] - AssertionError:  the results from backend jax and ground truth framework numpy do not match
FAILED ivy_tests/test_ivy/test_frontends/test_numpy/test_ndarray/test_ndarray.py::test_numpy_ndarray_prod[cpu-tensorflow-False-False] - AssertionError:  the results from backend tensorflow and ground truth framework numpy do no...
FAILED ivy_tests/test_ivy/test_frontends/test_numpy/test_ndarray/test_ndarray.py::test_numpy_ndarray_prod[cpu-torch-False-False] - AssertionError:  the results from backend torch and ground truth framework numpy do not match
==================== 4 failed, 1 skipped, 181 warnings in 65.18s (0:01:05) =====================

e.g.

E       AssertionError:  the results from backend tensorflow and ground truth framework numpy do not match
E        2!=1 
E       
E       
E       Falsifying example: test_numpy_ndarray_prod(
E           backend_fw='tensorflow',
E           frontend='numpy',
E           on_device='cpu',
E           dtype_x_axis_dtype=(['uint32'],
E            [array([0], dtype=uint32)],
E            0,
E            'uint32',
E            [array([False])]),
E           keep_dims=False,
E           initial=1.9999999701976776,
E           init_flags=FrontendMethodTestFlags(
E               num_positional_args=0,
E               as_variable=[False],
E               native_arrays=[False],
E           ),
E           method_flags=FrontendMethodTestFlags(
E               num_positional_args=0,
E               as_variable=[False],
E               native_arrays=[False],
E           ),
E           frontend_method_data=FrontendMethodData(ivy_init_module='ivy.functional.frontends.numpy', framework_init_module='numpy', init_name='array', method_name='prod'),
E       )
E       
E       You can reproduce this example by temporarily adding @reproduce_failure('6.82.5', b'AXicY2RABg1AzPj//38IDwAfKgOA') as a decorator on your test case


[ShreyanshBardia]

@Pchatain, that's a little weird, I re-ran the tests again and they seem to pass successfully, with only torch backend failing RuntimeError: "prod_out_cpu" not implemented for 'Half'

I'll update my branch once and run again to check if that's causing the issue

[ShreyanshBardia]

Hey @Pchatain, can you please check now? I am not sure why the tests not running accordingly at my end, if you have any idea kindly please do lemme know. Also I noticed a bug related to extreme float values, I think this will cause tests to fail

import ivy
initial = 1.99999997019767769

ivy.set_backend("numpy")
ivy.array(initial,dtype="int32")
#ivy.array(1)

ivy.set_backend("tensorflow")
ivy.array(initial,dtype="int32")
#ivy.array(2)

Is this expected? or should I look into it and try to solve, kindly guide

[Pchatain]

Hey @Pchatain, can you please check now? I am not sure why the tests not running accordingly at my end, if you have any idea kindly please do lemme know. Also I noticed a bug related to extreme float values, I think this will cause tests to fail

import ivy
initial = 1.99999997019767769

ivy.set_backend("numpy")
ivy.array(initial,dtype="int32")
#ivy.array(1)

ivy.set_backend("tensorflow")
ivy.array(initial,dtype="int32")
#ivy.array(2)

Is this expected? or should I look into it and try to solve, kindly guide

Hey so if you click on checks located at the top,

then you can click on re-run all tests to run the tests yourself.

I don't think this is expected, unless you observe the same behavior with torch itself for example. Could you investigate it a little bit?

[ShreyanshBardia]

Hi @Pchatain, sorry for the late reply. I don't think is an issue due to ivy's abstraction, this bug seems to exist in tensorflow framework itself.

import torch
import numpy as np
import tensorflow as tf

torch.tensor(1.999999999999999,dtype=torch.int64) #15 digits
#tensor(1)

tf.cast(1.99999999,dtype=tf.int64) #8 digits
#<tf.Tensor: shape=(), dtype=int64, numpy=2>

np.array(1.999999999999999,dtype=np.int64)
# array(1)

One way I can think of solving this is, when casting from float to int in case of tensorflow we should use numpy first and then convert it to tensorflow. What's your opinion? Also I'm not sure if everyone can re-run the tests, because I don't see any such feature, it might require priveleges to be granted

[Pchatain]

Hi @Pchatain, sorry for the late reply. I don't think is an issue due to ivy's abstraction, this bug seems to exist in tensorflow framework itself.

import torch
import numpy as np
import tensorflow as tf

torch.tensor(1.999999999999999,dtype=torch.int64) #15 digits
#tensor(1)

tf.cast(1.99999999,dtype=tf.int64) #8 digits
#<tf.Tensor: shape=(), dtype=int64, numpy=2>

np.array(1.999999999999999,dtype=np.int64)
# array(1)

The only way I can think of solving this is, when casting from float to int in case of tensorflow we should use numpy first and then convert it to tensorflow. What's your opinion? Also I'm not sure if everyone can re-run the tests, because I don't see any such feature, it might require priveleges to be granted

Sorry about the late reply, I mistakenly thought I had replied. Yes, that's entirely likely that I have to click on run tests. Let me know when you want to do this.

Thanks for looking into this. After reading more, it seems this is indeed a small quirk of the way floating points are handled. This is actually rather strange indeed consider https://www.tensorflow.org/api_docs/python/tf/cast suggests that cast should round down. Here is what I'm wondering:

1. Last week I saw more tests than just tensorflow failing such as AssertionError: the results from backend numpy and ground truth framework numpy do not match. Let me know if those are working and I can re-run the testing jobs.
2. I'm wondering if this is indeed exactly a problem with the frameworks or a problem with the way we are passing in the actual floats from hypothesis (hypothesis is what generates the testing cases). In this comment: fixed numpy.prod failing tests #21858 (comment), we are directly casting in the case of tensorflow, whereas the other modules are using array creation. Here is an example test

>>> a = tf.constant(1.99999999, dtype=tf.float64)
>>> tf.dtypes.cast(a, tf.int32)
# <tf.Tensor: shape=(), dtype=int32, numpy=1>

So that works, but using float32 as intermediary value immediately rounds it to 2.0.

>>> a = tf.constant(1.99999999, dtype=tf.float32)
>>> a
<tf.Tensor: shape=(), dtype=float32, numpy=2.0>
>>> tf.dtypes.cast(a, tf.int32)
<tf.Tensor: shape=(), dtype=int32, numpy=2>

[Pchatain]

@CatB1t could I get a second opinion on this? I'm not sure if the floating point mismatch requires a change in the way we test functions and what change to make if it does require one.

[ShreyanshBardia]

Hi @Pchatain, thanks for looking into this more. From what I understand I don't think this has anything to do with hypothesis, I believe we should update the array (tensor) creation for tensorflow backend. The below is the current implementation of asarray in tensorflow backend.

try:
    ret = tf.convert_to_tensor(obj, dtype)
except (TypeError, ValueError):
    ret = tf.cast(obj, dtype)
return tf.identity(ret) if copy else ret

The issue lies with using tf.cast directly, here's what the documentation says about tf.cast

Note this operation can lead to a loss of precision when converting native Python float and complex variables to [tf.float64](https://www.tensorflow.org/api_docs/python/tf#float64) or [tf.complex128](https://www.tensorflow.org/api_docs/python/tf#complex128) tensors, since the input is first converted to the float32 data type and then widened. It is recommended to use [tf.convert_to_tensor](https://www.tensorflow.org/api_docs/python/tf/convert_to_tensor) instead of [tf.cast](https://www.tensorflow.org/api_docs/python/tf/cast) for any non-tensor inputs.

I think something like this should solve the issue

try:
    ret = tf.convert_to_tensor(obj, dtype)
except (TypeError, ValueError):
    obj = tf.convert_to_tensor(obj, tf.float64)
    ret = tf.cast(obj, dtype)
return tf.identity(ret) if copy else ret

[Pchatain]

Hey @ShreyanshBardia I think that makes sense. The logic is basically if it fails, instead of directly casting we should take it to something high precision like float64 and then cast it. I'm thinking they probably made the choice to default to float32 because it leads to better performance? In any case, barring skipping the particular tests in hypothesis that are these extreme float values, I think it should maybe convert to complex128 and then convert down to dtype, since that will preserve the most information. If the tests pass with float64, feel free to do that exactly as you stated.

Indeed the problem definitely not with hypothesis itself, but the tests are selected by hypothesis I think. So hypothesis will generate these test cases that cause the failure.

[ShreyanshBardia]

Yes it seems so, both torch and tesorflow default to float32 (incase of floats). torch by default itself takes care of casting to any dtype at time of initialisation, whereas tensorflow allows casting to only superior dtypes, which was causing the buggy behaviour. I think float64 should be good enough, since complex128 is the superior dtype it shall not throw an error in its default casting. Also the below seems to work error-free so I see no point in using complex128 or complex64. Let me know if you can think of any other scenario, where it might be a problem. Also since the tests are passing I think it won't be an issue

import torch
import tensorflow as tf
import numpy as np

tf.convert_to_tensor([1j],dtype=tf.complex64)
tf.convert_to_tensor(torch.tensor([1j],dtype=torch.complex128),dtype=tf.complex64)
tf.convert_to_tensor(np.array([1j],dtype=np.complex128),dtype=tf.complex64)

Currently all tests are passing both np.prod and ndarray.prod, the onlt failing test is test_asarray, which I don't think is caused by this PR. The function doesn't seem to be buggy, it looks like there is some bug in the testing logic. My hypothesis is that tests are failing because the ivy.Shape was generated using a different backend and is then being passed on to tensorflow due to which the function is not working as expected. The PR looks good from my side, if you think any changes are required, do let me know. Thanks @Pchatain for following through!

[Pchatain]

Awesome @ShreyanshBardia, and thanks for your patience. Running the CI checks again and then will merge if those work.

[Pchatain]

Hey @ShreyanshBardia , just some minor linting errors. Could you run pre-commit and accept the changes it suggests? Then I'll merge.

[ShreyanshBardia]

ivy-gardener
✅ Ivy gardener has formatted your code.
If changes are requested, don't forget to pull your fork.

[ShreyanshBardia]

Hey @Pchatain, it looks like some other changes were made by bot too, should I remove those?

[Pchatain]

Hey @Pchatain, it looks like some other changes were made by bot too, should I remove those?

yeah reject the changes on the conflicted files you didn't edit.

[ShreyanshBardia]

Hey @Pchatain, I have removed the changes made by bot