Skip to content

Use BLAS_Order.ColMajor sklearn/utils/_cython_blas.pyx #31263

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

Merged
merged 3 commits into from
Apr 28, 2025

Conversation

ogrisel
Copy link
Member

@ogrisel ogrisel commented Apr 28, 2025

Attempt to fix #31257.

I also had a linter warning when editing this file in my IDE prior to making the changes in this PR.

I wonder why this problem would be free-threading specific though.

Copy link

github-actions bot commented Apr 28, 2025

✔️ Linting Passed

All linting checks passed. Your pull request is in excellent shape! ☀️

Generated for commit: d0f7a68. Link to the linter CI: here

@ogrisel
Copy link
Member Author

ogrisel commented Apr 28, 2025

So this PR seems to fix the original problem, but then I have the feeling that we are running into a deadlock later.

@ogrisel ogrisel force-pushed the ImportError-ColMajor branch from 50747fb to 3e74b36 Compare April 28, 2025 10:18
@ogrisel ogrisel force-pushed the ImportError-ColMajor branch from 3e74b36 to 339e422 Compare April 28, 2025 10:19
@ogrisel ogrisel marked this pull request as ready for review April 28, 2025 12:07
@ogrisel ogrisel added the Quick Review For PRs that are quick to review label Apr 28, 2025
@@ -88,6 +88,7 @@ jobs:
DISTRIB: 'conda-free-threaded'
LOCK_FILE: './build_tools/azure/pylatest_free_threaded_linux-64_conda.lock'
COVERAGE: 'false'
SKLEARN_FAULTHANDLER_TIMEOUT: '1800' # 30 * 60 seconds
Copy link
Member Author

@ogrisel ogrisel Apr 28, 2025

Choose a reason for hiding this comment

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

The usual duration for the pylatest_free_threaded run is below 20 min, so 30 minutes should give a large enough margin to avoid false positives.

@ogrisel
Copy link
Member Author

ogrisel commented Apr 28, 2025

The deadlock did not happen with the last commit, but if it happens again in the future, we should get more information to track down the cause of the problem, or at least point us to the test that is related to a potential race-condition.

@adrinjalali adrinjalali merged commit 3d64376 into scikit-learn:main Apr 28, 2025
42 checks passed
@ogrisel ogrisel deleted the ImportError-ColMajor branch April 29, 2025 08:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cython module:utils Quick Review For PRs that are quick to review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

⚠️ CI failed on Wheel builder (last failure: Apr 28, 2025) ⚠️
2 participants