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

[Fix] CuBIC, SPADE with scipy 1.14.0, deprecated .A attribute in scipy.sparse matrices` #636

Merged

Conversation

Moritz-Alexander-Kern
Copy link
Member

@Moritz-Alexander-Kern Moritz-Alexander-Kern commented Jun 28, 2024

Description

This PR addresses the issue arising from the deprecation of the .A attribute in scipy.sparse matrices starting with scipy==1.14.0 .

The .A attribute was used to convert sparse matrices to dense ndarray format.

However, this attribute was deprecated and has been removed in favor of the more explicit .toarray() method.

This deprecation was introduced with scipy==1.11.0. The PR removing it is #20499, see: scipy/scipy#20499

See also release notes: https://docs.scipy.org/doc/scipy/release/1.14.0-notes.html#expired-deprecations

Changes

SPADE

Replaced the use of .A with .toarray() in the _build_context function in the spade module.
This change preserves the functionality of converting a coo_matrix to a dense format without breaking the code due to the deprecated attribute.

This was done following the official recommendation, see property docstring:

    @property
    def A(self) -> np.ndarray:
        """DEPRECATED: Return a dense array.
        .. deprecated:: 1.11.0
            `.A` is deprecated and will be removed in v1.14.0.
            Use `.toarray()` instead.
        """
        if isinstance(self, sparray):
            message = ("`.A` is deprecated and will be removed in v1.14.0. "
                       "Use `.toarray()` instead.")
            warn(VisibleDeprecationWarning(message), stacklevel=2)
        return self.toarray()

Cubic

Ensure numeric precision of data ndarray is float64.

On Windows test fail with:

__________________________ CubicTestCase.test_cubic ___________________________

self = <elephant.test.test_cubic.CubicTestCase testMethod=test_cubic>

    def test_cubic(self):
    
        # Computing the output of CuBIC for the test data AnalogSignal
        xi, p_vals, k, test_aborted = cubic.cubic(
            self.data_signal, alpha=self.alpha)
    
        # Check the types of the outputs
        self.assertIsInstance(xi, int)
        self.assertIsInstance(p_vals, list)
        self.assertIsInstance(k, list)
    
        # Check that the number of tests is the output order of correlation
        self.assertEqual(xi, len(p_vals))
    
        # Check that all the first  xi-1 tests have not passed the
        # significance level alpha
        for p in p_vals[:-1]:
            self.assertGreater(self.alpha, p)
    
        # Check that the last p-value has passed the significance level
        self.assertGreater(p_vals[-1], self.alpha)
    
        # Check that the number of cumulant of the output is 3
        self.assertEqual(3, len(k))
    
        # Check the analytical constrain of the cumulants for which K_1<K_2
        self.assertGreater(k[1], k[0])
    
        # Check the computed order of correlation is the expected
        # from the test data
        self.assertEqual(xi, self.xi)
    
        # Computing the output of CuBIC for the test data Array
>       xi, p_vals, k, test_aborted = cubic.cubic(
            self.data_array, alpha=self.alpha)

elephant\test\test_cubic.py:74: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
elephant\cubic.py:142: in cubic
    pval = _H03xi(kappa, xi, L)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

kappa = [0.03, -0.1303980335803358, 3.000090626933894], xi = 1, L = 100000

    def _H03xi(kappa, xi, L):
        """
        Computes the p_value for testing  the :math:`H_0: k_3(data)<=k^*_{3,\\xi}`
        hypothesis of CuBIC in the stationary rate version
    
        Parameters
        -----
        kappa : list
            The first three cumulants of the populaton of spike trains
        xi : int
            The the maximum order of correlation :math:`\\xi` supposed in the
            hypothesis for which is computed the p value of :math:`H_0`
        L : float
            The length of the orginal population histogram on which is performed
            the CuBIC analysis
    
        Returns
        -----
        p : float
            The p-value of the hypothesis tests
        """
    
        # Check the order condition of the cumulants necessary to perform CuBIC
        if kappa[1] < kappa[0]:
>           raise ValueError(f"The null hypothesis H_0 cannot be tested: the "
                             f"population count histogram variance ({kappa[1]}) "
                             f"is less than the mean ({kappa[0]}). This can "
                             f"happen when the spike train population is not "
                             f"large enough or the bin size is small.")
E           ValueError: The null hypothesis H_0 cannot be tested: the population count histogram variance (-0.1303980335803358) is less than the mean (0.03). This can happen when the spike train population is not large enough or the bin size is small.

This seems similar to:
#227
#229

@coveralls
Copy link
Collaborator

Coverage Status

coverage: 88.222%. remained the same
when pulling e7cf9e0 on INM-6:fix/spade_with_scipy_1_14_0
into bdd98ee on NeuralEnsemble:master.

@Moritz-Alexander-Kern Moritz-Alexander-Kern changed the title [Fix] SPADE with scipy 1.14.0, deprecated .A attribute in scipy.sparse matrices` [Fix] Cubic, SPADE with scipy 1.14.0, deprecated .A attribute in scipy.sparse matrices` Jun 28, 2024
@Moritz-Alexander-Kern Moritz-Alexander-Kern changed the title [Fix] Cubic, SPADE with scipy 1.14.0, deprecated .A attribute in scipy.sparse matrices` [Fix] CuBIC, SPADE with scipy 1.14.0, deprecated .A attribute in scipy.sparse matrices` Jun 28, 2024
@coveralls
Copy link
Collaborator

Coverage Status

coverage: 87.892% (-0.3%) from 88.222%
when pulling c3a1aa6 on INM-6:fix/spade_with_scipy_1_14_0
into bdd98ee on NeuralEnsemble:master.

@coveralls
Copy link
Collaborator

Coverage Status

coverage: 88.222%. remained the same
when pulling c3a1aa6 on INM-6:fix/spade_with_scipy_1_14_0
into bdd98ee on NeuralEnsemble:master.

@coveralls
Copy link
Collaborator

Coverage Status

coverage: 87.892% (-0.3%) from 88.222%
when pulling 17d4609 on INM-6:fix/spade_with_scipy_1_14_0
into bdd98ee on NeuralEnsemble:master.

@coveralls
Copy link
Collaborator

Coverage Status

coverage: 88.222%. remained the same
when pulling 17d4609 on INM-6:fix/spade_with_scipy_1_14_0
into bdd98ee on NeuralEnsemble:master.

@Moritz-Alexander-Kern Moritz-Alexander-Kern requested review from mdenker and removed request for mdenker July 11, 2024 14:12
@coveralls
Copy link
Collaborator

coveralls commented Jul 15, 2024

Coverage Status

coverage: 88.222%. remained the same
when pulling c8fcf8f on INM-6:fix/spade_with_scipy_1_14_0
into bdd98ee on NeuralEnsemble:master.

Copy link
Member Author

@Moritz-Alexander-Kern Moritz-Alexander-Kern left a comment

Choose a reason for hiding this comment

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

build wheels

@Moritz-Alexander-Kern Moritz-Alexander-Kern merged commit 0984e19 into NeuralEnsemble:master Jul 17, 2024
15 checks passed
@Moritz-Alexander-Kern Moritz-Alexander-Kern added this to the v1.2.0 milestone Jul 17, 2024
@NeuralEnsemble NeuralEnsemble deleted a comment from coveralls Oct 21, 2024
@NeuralEnsemble NeuralEnsemble deleted a comment from coveralls Oct 21, 2024
@Moritz-Alexander-Kern Moritz-Alexander-Kern modified the milestones: v1.2.0, v1.1.1 Oct 24, 2024
Moritz-Alexander-Kern added a commit that referenced this pull request Oct 25, 2024
…cipy.sparse` matrices` (#636)

* fix deprecated .A attribute on coo_matrix
* make precision for cubic explicit
* ensure float64 precision for data when using scipy.stats.kstat
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants