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

Backport fix for flapack_sym_herm.pyf.src to mainatince/1.9.x #17224

Closed
thomasjpfan opened this issue Oct 13, 2022 · 8 comments
Closed

Backport fix for flapack_sym_herm.pyf.src to mainatince/1.9.x #17224

thomasjpfan opened this issue Oct 13, 2022 · 8 comments
Labels
maintenance Items related to regular maintenance tasks scipy.linalg
Milestone

Comments

@thomasjpfan
Copy link
Contributor

thomasjpfan commented Oct 13, 2022

While debugging a segfault in scikit-learn's test suite on Python 3.11 and Linux, I found that 292b643 is required to prevent the segfault. For testing, I cherry-picked the commit onto mainatince/1.9.x, build SciPy's wheel with cibuildwheel, and it resolve the segfault issue in scikit-learn's test suite.

Can SciPy backport 292b643 to mainatince/1.9.X?

Edit: For reference: this CI test run illustrates the error from Python. I get this backtrace when I have debug symbols on.

@thomasjpfan
Copy link
Contributor Author

For verification, I built SciPy 1.9.3 wheels with 292b643 cherry-picked, uploaded it to my own index and ran scikit-learn's test suite using my index. This resolves the segfault issue and all the tests pass!

@tylerjereddy tylerjereddy added this to the 1.9.3 milestone Oct 15, 2022
@tylerjereddy
Copy link
Contributor

@rgommers @ilayn what do you think? As far as I can tell I simply missed a ping in gh-16528 back in July. We've already got 8 merged backport candidates after labeling that one.. shall I aim for 1.9.3 soon? There's some manual artifact downloaing/unzipping and wheel renaming on maintenance/1.9.x for releases but the process is a fair bit faster than it used to be at least.

@rgommers
Copy link
Member

Yes, a 1.9.3 seems like a good idea. I think it'd be useful to do a scan through commits on main that weren't backported and see if anything else important may be missing. Not everyone is in the habit of thinking about backports when merging PRs.

@ilayn
Copy link
Member

ilayn commented Oct 15, 2022

My apologies @tylerjereddy I should have marked it as a candidate instead of a mere ping. The code change is harmless for us but clearly has certain implications for downstream so I'd say at your first convenience it would be nice to have this backported either 1.9.2 backport if there will be or 1.9.3 if not.

@ev-br ev-br added the backport-candidate This fix should be ported by a maintainer to previous SciPy versions. label Oct 19, 2022
@ev-br
Copy link
Member

ev-br commented Oct 19, 2022

@tylerjereddy am tentatively adding a backport-candidate label so that it does not fall through the cracks again :-).

@tylerjereddy
Copy link
Contributor

@ev-br I think we're good now--I already included the fix in #17239 right? 1.9.3 is on the way soon-ish

@ev-br ev-br removed the backport-candidate This fix should be ported by a maintainer to previous SciPy versions. label Oct 19, 2022
@ev-br
Copy link
Member

ev-br commented Oct 19, 2022

Sorry for the noise then

@rgommers
Copy link
Member

1.9.3 was released with the backport in it, so let's close this. Thanks all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Items related to regular maintenance tasks scipy.linalg
Projects
None yet
Development

No branches or pull requests

5 participants