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

MNT Update dependencies #118

Merged
merged 2 commits into from
Oct 5, 2023
Merged

MNT Update dependencies #118

merged 2 commits into from
Oct 5, 2023

Conversation

fcharras
Copy link
Collaborator

@fcharras fcharras commented Oct 5, 2023

Update Dockerfile, patch around inline optimization issues, and hack around drop of atomics on CPU to keep a working CI (by pinning CI on an old working version).

…around drop of atomics on CPU to keep a working CI (by pinning CI on an old working version).
Copy link
Collaborator

@ogrisel ogrisel left a comment

Choose a reason for hiding this comment

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

If it works, LGTM :)

# Version of other python packages explicitly installed either within the
# build environment or the runtime environment.

ARG NUMPY_VERSION="<1.25"
ARG NUMBA_VERSION=""
ARG CYTHON_VERSION="<3"
ARG CYTHON_VERSION=""
Copy link
Collaborator

Choose a reason for hiding this comment

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

If Cython is used to build scikit-learn, note that there are a few performance regressions with Cython 3. So we might want to stick to the latest Cython 2 for the time being.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

cython is not installed in the final image, so it's not related to the cython one can use to build sklearn later on in a container. Most recent dpctl has merged work that makes it compatible with cython>=3 so I think we can keep that.

@ogrisel
Copy link
Collaborator

ogrisel commented Oct 5, 2023

Apparently it doesn't work:

WARNING: The directory '/github/home/.cache/pip' or its parent directory is not owned or is not writable by the current user. The cache has been disabled. Check the permissions and owner of that directory. If executing pip with sudo, you should use sudo's -H flag.
WARNING: Requirement '/opt/numba_dpex/cpu-stable/numba_dpex*.whl' looks like a filename, but the file does not exist
ERROR: numba_dpex*.whl is not a valid wheel filename.

@fcharras
Copy link
Collaborator Author

fcharras commented Oct 5, 2023

It's normal that the test don't pass yet, we must merge first so that the runner build and update the image to our repo, and re-run the pipelines after that. I'll perform a few final tests locally before.

@fcharras
Copy link
Collaborator Author

fcharras commented Oct 5, 2023

TY for review

@fcharras fcharras merged commit d5c551c into main Oct 5, 2023
1 of 2 checks passed
@fcharras fcharras deleted the mnt_bump_to_latest_versions branch October 5, 2023 21:56
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.

2 participants