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

update assert np.allclose to numpy.testing.assert_allclose in tests #1904

Open
wants to merge 1 commit into
base: v4-dev
Choose a base branch
from

Conversation

VeckoTheGecko
Copy link
Contributor

  • Chose the correct base branch (main for v3 changes, v4-dev for v4 changes)

numpy.testing.assert_allclose has a better Pytest error message.

Before:

>       assert np.allclose(pset.u, lat, rtol=1e-6)
E       assert False
E        +  where False = <function allclose at 0x1034aa6f0>(array([-80.        , -78.655464  , -77.31092   , -75.966385  ,\n       -74.62184   , -73.27731   , -71.93277   , -70.58...77   ,  73.27731   ,  74.62185   ,\n        75.966385  ,  77.310905  ,  78.65546   ,  80.        ],\n      dtype=float32), array([-80.        , -78.65546218, -77.31092437, -75.96638655,\n       -74.62184874, -73.27731092, -71.93277311, -70.58... 70.58823529,  71.93277311,  73.27731092,  74.62184874,\n        75.96638655,  77.31092437,  78.65546218,  80.        ]), rtol=1e-06)
E        +    where <function allclose at 0x1034aa6f0> = np.allclose
E        +    and   array([-80.        , -78.655464  , -77.31092   , -75.966385  ,\n       -74.62184   , -73.27731   , -71.93277   , -70.58...77   ,  73.27731   ,  74.62185   ,\n        75.966385  ,  77.310905  ,  78.65546   ,  80.        ],\n      dtype=float32) = <ParticleSet>\n    fieldset   :\n        <FieldSet>\n            fields:\n                <Field>\n                    name...n=-45.000000, lat=-71.932770, depth=0.000000, u=-71.932770, v=-45.000000, p=0.000000, time=0.000000),\n        ...\n    ].u

tests/test_fieldset_sampling.py:408: AssertionError

After:

>       assert_allclose(pset.u, lat, rtol=1e-6)

tests/test_fieldset_sampling.py:408: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

args = (<function assert_allclose.<locals>.compare at 0x15e078b80>, array([-80.        , -78.655464  , -77.31092   , -75.9663...70.58823529,  71.93277311,  73.27731092,  74.62184874,
        75.96638655,  77.31092437,  78.65546218,  80.        ]))
kwds = {'equal_nan': True, 'err_msg': '', 'header': 'Not equal to tolerance rtol=1e-06, atol=0', 'strict': False, ...}

    @wraps(func)
    def inner(*args, **kwds):
        with self._recreate_cm():
>           return func(*args, **kwds)
E           AssertionError: 
E           Not equal to tolerance rtol=1e-06, atol=0
E           
E           Mismatched elements: 2 / 120 (1.67%)
E           Max absolute difference among violations: 4.72830123e-06
E           Max relative difference among violations: 7.03334809e-06
E            ACTUAL: array([-80.      , -78.655464, -77.31092 , -75.966385, -74.62184 ,
E                  -73.27731 , -71.93277 , -70.588234, -69.24369 , -67.89917 ,
E                  -66.55461 , -65.210075, -63.865543, -62.521004, -61.176468,...
E            DESIRED: array([-80.      , -78.655462, -77.310924, -75.966387, -74.621849,
E                  -73.277311, -71.932773, -70.588235, -69.243697, -67.89916 ,
E                  -66.554622, -65.210084, -63.865546, -62.521008, -61.176471,...

../../../miniforge3/envs/parcels-dev/lib/python3.10/contextlib.py:79: AssertionError
--------------------------------------------------------------------------------------

@VeckoTheGecko
Copy link
Contributor Author

Docs on numpy.testing.assert_allclose

The test is equivalent to allclose(actual, desired, rtol, atol) (note that allclose has different default values).

This might cause failures in the test suite. Let's wait for CI

Copy link
Member

@erikvansebille erikvansebille left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice improvement, @VeckoTheGecko! I didn't know that this function existed, but I'm sure it'll make development against unit tests easier!

Also implement this change in the docs/examples/* files?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Backlog
Development

Successfully merging this pull request may close these issues.

2 participants