Skip to content

TST: Non-deterministic failures with AssertionError: Value of _parameters changed by pickling #18569

New issue
New issue
Closed
#18596
Closed
TST: Non-deterministic failures with AssertionError: Value of _parameters changed by pickling#18569
#18596
Labels
Bugconvolutionmodelingtesting

Description

@pllim
pllim
opened on Aug 29, 2025

Only shows up once in a while and usually goes away on rerun, but still, this should not happen in the first place.

____________________ test_simple_object[0-Gaussian1DKernel] ____________________

pickle_protocol = 0
original = <astropy.convolution.kernels.Gaussian1DKernel object at 0x7fb0f429fcb0>

    @pytest.mark.parametrize(
        "original",
        [
            conv.CustomKernel(array=np.random.rand(15)),
            conv.Gaussian1DKernel(1.0, x_size=5),
            conv.Gaussian2DKernel(1.0, x_size=5, y_size=5),
        ],
        ids=lambda x: type(x).__name__,
    )
    def test_simple_object(pickle_protocol, original):  # noqa: F811
        # Tests easily instantiated objects
>       check_pickling_recovery(original, pickle_protocol)

../../.tox/py313-test-devdeps/lib/python3.13/site-packages/astropy/convolution/tests/test_pickle.py:21: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
../../.tox/py313-test-devdeps/lib/python3.13/site-packages/astropy/tests/helper.py:167: in check_pickling_recovery
    generic_recursive_equality_test(original, unpickled, class_history)
../../.tox/py313-test-devdeps/lib/python3.13/site-packages/astropy/tests/helper.py:154: in generic_recursive_equality_test
    generic_recursive_equality_test(
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

a = <Gaussian1D(amplitude=0.39894228, mean=0., stddev=1.)>
b = <Gaussian1D(amplitude=0.39894228, mean=0., stddev=1.)>
class_history = [<class 'astropy.modeling.functional_models.Gaussian1D'>
Name: Gaussian1D
N_inputs: 1
N_outputs: 1
Fittable parameters: ('amplitude', 'mean', 'stddev'), <class 'astropy.convolution.kernels.Gaussian1DKernel'>]

    def generic_recursive_equality_test(a, b, class_history):
        """
        Check if the attributes of a and b are equal. Then,
        check if the attributes of the attributes are equal.
        """
        # NOTE: The call may need to be adapted if other objects implementing a __getstate__
        # with required argument(s) are passed to this function.
        # For a class with `__slots__` the default state is not a `dict`;
        # with neither `__dict__` nor `__slots__` it is `None`.
        state = a.__getstate__(a) if isinstance(a, type) else a.__getstate__()
        dict_a = state if isinstance(state, dict) else getattr(a, "__dict__", {})
        dict_b = b.__dict__
        for key in dict_a:
            assert key in dict_b, f"Did not pickle {key}"
    
            if dict_a[key].__class__.__eq__ is not object.__eq__:
                # Only compare if the class defines a proper equality test.
                # E.g., info does not define __eq__, and hence defers to
                # object.__eq__, which is equivalent to checking that two
                # instances are the same.  This will generally not be true
                # after pickling.
                eq = dict_a[key] == dict_b[key]
                if "__iter__" in dir(eq):
                    eq = False not in eq
>               assert eq, f"Value of {key} changed by pickling"
                       ^^
E               AssertionError: Value of _parameters changed by pickling

../../.tox/py313-test-devdeps/lib/python3.13/site-packages/astropy/tests/helper.py:145: AssertionError
____________________ test_simple_object[1-Gaussian1DKernel] ____________________

pickle_protocol = 1
original = <astropy.convolution.kernels.Gaussian1DKernel object at 0x7fb0f429fcb0>

    @pytest.mark.parametrize(
        "original",
        [
            conv.CustomKernel(array=np.random.rand(15)),
            conv.Gaussian1DKernel(1.0, x_size=5),
            conv.Gaussian2DKernel(1.0, x_size=5, y_size=5),
        ],
        ids=lambda x: type(x).__name__,
    )
    def test_simple_object(pickle_protocol, original):  # noqa: F811
        # Tests easily instantiated objects
>       check_pickling_recovery(original, pickle_protocol)

../../.tox/py313-test-devdeps/lib/python3.13/site-packages/astropy/convolution/tests/test_pickle.py:21: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
../../.tox/py313-test-devdeps/lib/python3.13/site-packages/astropy/tests/helper.py:167: in check_pickling_recovery
    generic_recursive_equality_test(original, unpickled, class_history)
../../.tox/py313-test-devdeps/lib/python3.13/site-packages/astropy/tests/helper.py:154: in generic_recursive_equality_test
    generic_recursive_equality_test(
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

a = <Gaussian1D(amplitude=0.39894228, mean=0., stddev=1.)>
b = <Gaussian1D(amplitude=0.39894228, mean=0., stddev=1.)>
class_history = [<class 'astropy.modeling.functional_models.Gaussian1D'>
Name: Gaussian1D
N_inputs: 1
N_outputs: 1
Fittable parameters: ('amplitude', 'mean', 'stddev'), <class 'astropy.convolution.kernels.Gaussian1DKernel'>]

    def generic_recursive_equality_test(a, b, class_history):
        """
        Check if the attributes of a and b are equal. Then,
        check if the attributes of the attributes are equal.
        """
        # NOTE: The call may need to be adapted if other objects implementing a __getstate__
        # with required argument(s) are passed to this function.
        # For a class with `__slots__` the default state is not a `dict`;
        # with neither `__dict__` nor `__slots__` it is `None`.
        state = a.__getstate__(a) if isinstance(a, type) else a.__getstate__()
        dict_a = state if isinstance(state, dict) else getattr(a, "__dict__", {})
        dict_b = b.__dict__
        for key in dict_a:
            assert key in dict_b, f"Did not pickle {key}"
    
            if dict_a[key].__class__.__eq__ is not object.__eq__:
                # Only compare if the class defines a proper equality test.
                # E.g., info does not define __eq__, and hence defers to
                # object.__eq__, which is equivalent to checking that two
                # instances are the same.  This will generally not be true
                # after pickling.
                eq = dict_a[key] == dict_b[key]
                if "__iter__" in dir(eq):
                    eq = False not in eq
>               assert eq, f"Value of {key} changed by pickling"
                       ^^
E               AssertionError: Value of _parameters changed by pickling

../../.tox/py313-test-devdeps/lib/python3.13/site-packages/astropy/tests/helper.py:145: AssertionError
____________________ test_simple_object[4-Gaussian1DKernel] ____________________

pickle_protocol = 4
original = <astropy.convolution.kernels.Gaussian1DKernel object at 0x7fb0f429fcb0>

    @pytest.mark.parametrize(
        "original",
        [
            conv.CustomKernel(array=np.random.rand(15)),
            conv.Gaussian1DKernel(1.0, x_size=5),
            conv.Gaussian2DKernel(1.0, x_size=5, y_size=5),
        ],
        ids=lambda x: type(x).__name__,
    )
    def test_simple_object(pickle_protocol, original):  # noqa: F811
        # Tests easily instantiated objects
>       check_pickling_recovery(original, pickle_protocol)

../../.tox/py313-test-devdeps/lib/python3.13/site-packages/astropy/convolution/tests/test_pickle.py:21: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
../../.tox/py313-test-devdeps/lib/python3.13/site-packages/astropy/tests/helper.py:167: in check_pickling_recovery
    generic_recursive_equality_test(original, unpickled, class_history)
../../.tox/py313-test-devdeps/lib/python3.13/site-packages/astropy/tests/helper.py:154: in generic_recursive_equality_test
    generic_recursive_equality_test(
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

a = <Gaussian1D(amplitude=0.39894228, mean=0., stddev=1.)>
b = <Gaussian1D(amplitude=0.39894228, mean=0., stddev=1.)>
class_history = [<class 'astropy.modeling.functional_models.Gaussian1D'>
Name: Gaussian1D
N_inputs: 1
N_outputs: 1
Fittable parameters: ('amplitude', 'mean', 'stddev'), <class 'astropy.convolution.kernels.Gaussian1DKernel'>]

    def generic_recursive_equality_test(a, b, class_history):
        """
        Check if the attributes of a and b are equal. Then,
        check if the attributes of the attributes are equal.
        """
        # NOTE: The call may need to be adapted if other objects implementing a __getstate__
        # with required argument(s) are passed to this function.
        # For a class with `__slots__` the default state is not a `dict`;
        # with neither `__dict__` nor `__slots__` it is `None`.
        state = a.__getstate__(a) if isinstance(a, type) else a.__getstate__()
        dict_a = state if isinstance(state, dict) else getattr(a, "__dict__", {})
        dict_b = b.__dict__
        for key in dict_a:
            assert key in dict_b, f"Did not pickle {key}"
    
            if dict_a[key].__class__.__eq__ is not object.__eq__:
                # Only compare if the class defines a proper equality test.
                # E.g., info does not define __eq__, and hence defers to
                # object.__eq__, which is equivalent to checking that two
                # instances are the same.  This will generally not be true
                # after pickling.
                eq = dict_a[key] == dict_b[key]
                if "__iter__" in dir(eq):
                    eq = False not in eq
>               assert eq, f"Value of {key} changed by pickling"
                       ^^
E               AssertionError: Value of _parameters changed by pickling

../../.tox/py313-test-devdeps/lib/python3.13/site-packages/astropy/tests/helper.py:145: AssertionError
___________________ test_simple_object[-1-Gaussian1DKernel] ____________________

pickle_protocol = -1
original = <astropy.convolution.kernels.Gaussian1DKernel object at 0x7fb0f429fcb0>

    @pytest.mark.parametrize(
        "original",
        [
            conv.CustomKernel(array=np.random.rand(15)),
            conv.Gaussian1DKernel(1.0, x_size=5),
            conv.Gaussian2DKernel(1.0, x_size=5, y_size=5),
        ],
        ids=lambda x: type(x).__name__,
    )
    def test_simple_object(pickle_protocol, original):  # noqa: F811
        # Tests easily instantiated objects
>       check_pickling_recovery(original, pickle_protocol)

../../.tox/py313-test-devdeps/lib/python3.13/site-packages/astropy/convolution/tests/test_pickle.py:21: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
../../.tox/py313-test-devdeps/lib/python3.13/site-packages/astropy/tests/helper.py:167: in check_pickling_recovery
    generic_recursive_equality_test(original, unpickled, class_history)
../../.tox/py313-test-devdeps/lib/python3.13/site-packages/astropy/tests/helper.py:154: in generic_recursive_equality_test
    generic_recursive_equality_test(
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

a = <Gaussian1D(amplitude=0.39894228, mean=0., stddev=1.)>
b = <Gaussian1D(amplitude=0.39894228, mean=0., stddev=1.)>
class_history = [<class 'astropy.modeling.functional_models.Gaussian1D'>
Name: Gaussian1D
N_inputs: 1
N_outputs: 1
Fittable parameters: ('amplitude', 'mean', 'stddev'), <class 'astropy.convolution.kernels.Gaussian1DKernel'>]

    def generic_recursive_equality_test(a, b, class_history):
        """
        Check if the attributes of a and b are equal. Then,
        check if the attributes of the attributes are equal.
        """
        # NOTE: The call may need to be adapted if other objects implementing a __getstate__
        # with required argument(s) are passed to this function.
        # For a class with `__slots__` the default state is not a `dict`;
        # with neither `__dict__` nor `__slots__` it is `None`.
        state = a.__getstate__(a) if isinstance(a, type) else a.__getstate__()
        dict_a = state if isinstance(state, dict) else getattr(a, "__dict__", {})
        dict_b = b.__dict__
        for key in dict_a:
            assert key in dict_b, f"Did not pickle {key}"
    
            if dict_a[key].__class__.__eq__ is not object.__eq__:
                # Only compare if the class defines a proper equality test.
                # E.g., info does not define __eq__, and hence defers to
                # object.__eq__, which is equivalent to checking that two
                # instances are the same.  This will generally not be true
                # after pickling.
                eq = dict_a[key] == dict_b[key]
                if "__iter__" in dir(eq):
                    eq = False not in eq
>               assert eq, f"Value of {key} changed by pickling"
                       ^^
E               AssertionError: Value of _parameters changed by pickling

../../.tox/py313-test-devdeps/lib/python3.13/site-packages/astropy/tests/helper.py:145: AssertionError

Any ideas, @larrybradley or @keflavich ?

Activity

  1. pllim
    added
    testing
    Bug
    convolution
    on Aug 29, 2025

  2. keflavich commented on Aug 29, 2025

    @keflavich
    keflavich
    on Aug 29, 2025
    Contributor

    No, I'm out of my depth here, sorry. I don't understand what the test is really testing - I don't know of any reason the convolution kernels would have this problem. Is this on CI only, or also local?

  3. pllim commented on Aug 29, 2025

    @pllim
    pllim
    on Aug 29, 2025
    MemberAuthor

    Hmm looks like it was added back in #1947 , which mentioned #1942 . Pinging the people who thought this test was cool and still active: @taldcroft @astrofrog @eteq

  4. pllim commented on Aug 29, 2025

    @pllim
    pllim
    on Aug 29, 2025
    MemberAuthor

    Or maybe we revert #18453 , @eerovaher ?

  5. eerovaher commented on Sep 2, 2025

    @eerovaher
    eerovaher
    on Sep 2, 2025
    Member

    You can see from the tracebacks that the real problem is not in convolution, it is in modeling. Although modeling does test pickling, the tests there do not assert that the unpickled data matches the original exactly. Instead the tests use numpy.testing.assert_allclose(), which allows small differences.

    1. Why does unpickling models (sometimes) not recreate the original exactly?
    2. Why don't modeling pickling tests make use of astropy.tests.helper.check_pickling_recovery(), which would have revealed 1) much earlier?

    Given that the convolution kernels depend on modeling, there might not be too much value in testing pickling them if modeling is known to be problematic.

  6. eerovaher
    added
    modeling
    on Sep 2, 2025

  7. larrybradley commented on Sep 2, 2025

    @larrybradley
    larrybradley
    on Sep 2, 2025
    Member

    CC: @WilliamJamieson, @nden

  8. pllim commented on Sep 2, 2025

    @pllim
    pllim
    on Sep 2, 2025
    MemberAuthor

    Hopefully we can resolve soon? If not, can we just not run those tests? The intermittent failures do block required job, which blocks unrelated PRs, and it is getting annoying. It is intermittent but not rare.

  9. WilliamJamieson commented on Sep 2, 2025

    @WilliamJamieson
    WilliamJamieson
    on Sep 2, 2025
    Contributor

    When did this start happening? As far as I can remember no major changes have been made to how model parameters are being handled.

  10. pllim commented on Sep 2, 2025

    @pllim
    pllim
    on Sep 2, 2025
    MemberAuthor

    Not sure. Those tests were xfail until #18453

  11. larrybradley
    closed this as completedin #18596on Sep 5, 2025

  12. pllim commented on Sep 5, 2025

    @pllim
    pllim
    on Sep 5, 2025
    MemberAuthor

    Follow up issue at #18598

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    Bugconvolutionmodelingtesting

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions