scipy: BUG: memoryview error with Cython 0.29.31

Cython 0.29.31 was release a few hours ago and it corresponds to our CI going red.

https://github.com/cython/cython/releases/tag/0.29.31

See for instance: https://github.com/scipy/scipy/runs/7551272609?check_suite_focus=true

Error compiling Cython file:
1453
------------------------------------------------------------
1454
...
1455
    return np.array(result, dtype=np.int64)
1456

1457

1458
@cython.wraparound(False)
1459
@cython.boundscheck(False)
1460
def _weightedrankedtau(ordered[:] x, ordered[:] y, intp_t[:] rank, weigher, bool additive):
1461
^
1462
------------------------------------------------------------
1463

1464
_stats.pyx:173:0: Referring to a memoryview typed argument directly in a nested closure function is not supported in Cython 0.x. Either upgrade to Cython 3, or assign the argument to a local variable and use that in the nested function.

About this issue

Original URL
State: closed
Created 2 years ago
Comments: 17 (13 by maintainers)

Commits related to this issue

MAINT: stats: Work around Cython bug. Because of a bug in Cython, the inner function weigh() (a closure in _weightedrankedtau()) cannot refer to the memoryview arguments of _weightedrankedtau(). The... — committed to WarrenWeckesser/scipy by WarrenWeckesser 2 years ago
Use SciPy 1.9 RC to mitigate Cython-related build failure. - https://github.com/scipy/scipy/issues/16718 — committed to xkszltl/Roaster by xkszltl 2 years ago

Most upvoted comments

@xkszltl we are about to release 1.9 so if you can wait a few days that would be it. Otherwise you can use nightly builds we are making. But you might need to wait a bit until this PR gets into the nightly (not sure when is the rebuild happening).

tupui on Jul 28, 2022

Thanks @jjerphan. Warren already has a fix, I am just waiting for the CI to finish to get it in as it break all workflows. So we better fix this quick and if there are improvements to make later, we can still iterate 😃

tupui on Jul 28, 2022

@tupui: Thanks. scikit-learn has the same problem. I will resolve the issue there first, and I might inspect this issue then.

jjerphan on Jul 28, 2022

We’re talking about this inner function, apparently: https://github.com/scipy/scipy/blob/7f941dbc76ad90aa4dc23b613f5c2da454115075/scipy/stats/_stats.pyx#L235

IIUC, it uses the outer function’s memoryview arguments y and rank, but not x. Due to the bug in Cython 0.29.x, this is generally unsafe. But you can just assign the arguments to local variables and use those in the inner function. Or pass the outer arguments explicitly as inner arguments, but local variables seem simpler here. (Although it would be interesting to know if avoiding a closure has a performance impact.)

Cython 3.0 has a proper fix, but that’s not easy to backport. Thus the error in 0.29.31. Sorry for the hassle.

scoder on Jul 28, 2022

At the moment, I’m mostly speculating, so take this with a grain of salt…

Do we then just need to put this function outside?

If my understanding of the problem is correct, then yes, I think that would be one way to fix it. Move weigh out of _weightedrankedtau, and add all the variables that it needs from within the scope of _weightedrankedtau as additional parameters.

That might be more than necessary, however. The error message says

_stats.pyx:173:0: Referring to a memoryview typed argument directly in a nested closure
function is not supported in Cython 0.x. Either upgrade to Cython 3, or assign the argument
to a local variable and use that in the nested function.

Maybe the “memoryview typed argument” only refers to y and rank, which are arguments of _weightedrankedtau that are also used in weigh(). So it might be sufficient to leave weigh where it is, and just add y and rank as parameters instead of implicitly capturing it in the closure, or perhaps, as suggested in the error message, define a local variables in weigh and assign y and rank to them (but I don’t know enough about what Cython is doing to know if and why that would actually avoid the problem).

If any Cython devs/gurus happen to stop by, maybe they can suggest the simplest fix.

WarrenWeckesser on Jul 28, 2022

@jjerphan this might interest you.

tupui on Jul 28, 2022