Skip to content

Change equality to allclose to fix test on 32 bit#11881

Merged
pllim merged 1 commit into
astropy:mainfrom
mhvk:coordinates-fix-test
Jun 23, 2021
Merged

Change equality to allclose to fix test on 32 bit#11881
pllim merged 1 commit into
astropy:mainfrom
mhvk:coordinates-fix-test

Conversation

@mhvk

@mhvk mhvk commented Jun 23, 2021

Copy link
Copy Markdown
Contributor

@pllim

pllim commented Jun 23, 2021

Copy link
Copy Markdown
Member

Alas, there is a failure still.

________ TestArithmetic.test_add_sub[PhysicsSphericalRepresentation] __________
[gw3] linux -- Python 3.7.10 /__w/astropy/astropy/.tox/py37-test/bin/python

self = <astropy.coordinates.tests.test_representation_arithmetic.TestArithmetic object at 0xea71c9ec>
representation = <class 'astropy.coordinates.representation.PhysicsSphericalRepresentation'>

    @pytest.mark.parametrize('representation',
                             (PhysicsSphericalRepresentation,
                              SphericalRepresentation,
                              CylindricalRepresentation))
    def test_add_sub(self, representation):
        in_rep = self.cartesian.represent_as(representation)
        r1 = in_rep + in_rep
        assert isinstance(r1, representation)
        expected = 2. * in_rep
        for component in in_rep.components:
            assert_quantity_allclose(getattr(r1, component),
                                     getattr(expected, component))
        with pytest.raises(TypeError):
            10.*u.m + in_rep
        with pytest.raises(u.UnitsError):
            in_rep + (in_rep / u.s)
        r2 = in_rep - in_rep
        assert isinstance(r2, representation)
        assert_representation_allclose(
>           r2.to_cartesian(), CartesianRepresentation(0.*u.m, 0.*u.m, 0.*u.m))

../../.tox/py37-test/lib/python3.7/site-packages/astropy/coordinates/tests/test_representation_arithmetic.py:218: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
../../.tox/py37-test/lib/python3.7/site-packages/astropy/coordinates/tests/test_representation_arithmetic.py:27: in assert_representation_allclose
    assert_quantity_allclose(actual_xyz, desired_xyz, rtol, atol, **kwargs)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

actual = <Quantity [[0.00000000e+00, 0.00000000e+00, 0.00000000e+00],
           [0.00000000e+00, 0.00000000e+00, 0.00000000e+0...    [2.71917234e-32, 4.44089210e-16, 2.71917234e-32],
           [0.00000000e+00, 0.00000000e+00, 0.00000000e+00]] kpc>
desired = <Quantity [[0., 0., 0.],
           [0., 0., 0.],
           [0., 0., 0.],
           [0., 0., 0.],
           [0., 0., 0.],
           [0., 0., 0.],
           [0., 0., 0.]] m>
rtol = 1e-07, atol = None, kwargs = {}
np = <module 'numpy' from '/__w/astropy/astropy/.tox/py37-test/lib/python3.7/site-packages/numpy/__init__.py'>
_unquantify_allclose_arguments = <function _unquantify_allclose_arguments at 0xf1122c44>

    def assert_quantity_allclose(actual, desired, rtol=1.e-7, atol=None,
                                 **kwargs):
        """
        Raise an assertion if two objects are not equal up to desired tolerance.
    
        This is a :class:`~astropy.units.Quantity`-aware version of
        :func:`numpy.testing.assert_allclose`.
        """
        import numpy as np
        from astropy.units.quantity import _unquantify_allclose_arguments
        np.testing.assert_allclose(*_unquantify_allclose_arguments(
>           actual, desired, rtol, atol), **kwargs)
E       AssertionError: 
E       Not equal to tolerance rtol=1e-07, atol=0
E       
E       Mismatched elements: 3 / 21 (14.3%)
E       Max absolute difference: 4.4408921e-16
E       Max relative difference: inf
E        x: array([[0.000000e+00, 0.000000e+00, 0.000000e+00],
E              [0.000000e+00, 0.000000e+00, 0.000000e+00],
E              [0.000000e+00, 0.000000e+00, 0.000000e+00],...
E        y: array([[0., 0., 0.],
E              [0., 0., 0.],
E              [0., 0., 0.],...

@pllim pllim modified the milestones: v4.3, v4.3.1 Jun 23, 2021
@mhvk mhvk force-pushed the coordinates-fix-test branch from fead444 to fd71776 Compare June 23, 2021 01:15
@mhvk

mhvk commented Jun 23, 2021

Copy link
Copy Markdown
Contributor Author

Ah, yes, comparing with 0 with atol=0 is not going to help, unless rtol is very large! Hopefully fixed now...

@nstarman nstarman left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I see from the error messages that the offending differences were of order 1e-16, but consistently at the same index. It must be some special r2...

@pllim pllim left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Seems uncontroversial enough. Thanks!

p.s. I didn't even know assert_representation_allclose existed. TIL!

@pllim pllim merged commit 20249ba into astropy:main Jun 23, 2021
@pllim

pllim commented Jun 23, 2021

Copy link
Copy Markdown
Member

p.p.s. Oh gawd... look like the backport bot takes the branch name from description, not the label name.

@meeseeksdev backport to v4.3.x

meeseeksmachine pushed a commit to meeseeksmachine/astropy that referenced this pull request Jun 23, 2021
@pllim pllim added the testing label Jun 23, 2021
@mhvk mhvk deleted the coordinates-fix-test branch June 23, 2021 13:47
mhvk added a commit that referenced this pull request Jun 23, 2021
…881-on-v4.3.x

Backport PR #11881 on branch v4.3.x (Change equality to allclose to fix test on 32 bit)
astrofrog pushed a commit that referenced this pull request Aug 17, 2021
Change equality to allclose to fix test on 32 bit
@astrofrog astrofrog modified the milestones: v4.3.1, v4.0.6 Aug 17, 2021
@astrofrog

Copy link
Copy Markdown
Member

This is needed in LTS so changing the milestone

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TST: test_representation_arithmetic failures on 32-bit/parallel job with numpy 1.21.0

4 participants