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

Use numpy.testing.assert_allclose for more consistent floating-point comparisons #4488

Open
agriyakhetarpal opened this issue Oct 3, 2024 · 5 comments
Assignees
Labels
difficulty: easy A good issue for someone new. Can be done in a few hours priority: low No existing plans to resolve

Comments

@agriyakhetarpal
Copy link
Member

Currently, we use np.testing.assert_array_almost_equal in 404 places across 41 files, but the NumPy authors no longer recommend it. We should switch to the recommended np.testing.assert_allclose testing method where we can (which can be in one PR or multiple PRs), across all our tests.

Please note that there are a few differences in the number of arguments both functions take and in the default tolerance(s) set for them, so it's not a quick swap and some failures could be expected – but it should still be easy for someone to take it up in parts.

@agriyakhetarpal agriyakhetarpal added difficulty: easy A good issue for someone new. Can be done in a few hours priority: low No existing plans to resolve hacktoberfest labels Oct 3, 2024
@Saransh-cpp
Copy link
Member

Also, all the instances of assert all(numpy_array == numpy_array)/assert np.allclose(numpy_array == numpy_array) (if we have any) should be replaced with numpy.testing calls.

@mukulbindal
Copy link

Hi @Saransh-cpp @agriyakhetarpal If no one is working on this issue, kindly assign this to me.

@arjxn-py
Copy link
Member

Hi @mukulbindal, thanks for showing your interest. Feel free to pick this up and do let us know in case you need help :)

@agriyakhetarpal
Copy link
Member Author

Hi @mukulbindal, how is this coming along? Here's our guide in case you're not comfortable with contributing yet and if you need more instructions to follow along: https://docs.pybamm.org/en/latest/source/user_guide/contributing.html

@mukulbindal
Copy link

@agriyakhetarpal I got some personal emergency so I parked this task. I'll start on this now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
difficulty: easy A good issue for someone new. Can be done in a few hours priority: low No existing plans to resolve
Projects
None yet
Development

No branches or pull requests

4 participants