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

[CI, enhancement] add sklearnex CI job with nightly oneDAL DPCPP build in github actions #1845

Merged
merged 65 commits into from
Aug 29, 2024

Conversation

icfaust
Copy link
Contributor

@icfaust icfaust commented May 24, 2024

Description

This PR expands sklearnex testing to work with latest oneDAL builds. It uses a custom Visual Studio / Intel DPC++ compiler (AVX2) windows build for Windows testing. It uses a Intel DPC++ Linux build (AVX2). This uses only publicly available components (which would match external contributor capabilities), and follows what an external contributor would have to do to hand-build oneDAL/sklearnex.

It has:

  1. a nightly build version of oneDAL/main for testing without waiting for release
  2. additional DPCtl/dpnp windows testing
  3. a venv/pip build strategy for expanding robustness in our build processes
  4. building onedal with gcc in linux and vc in windows (use as much out-of-the-box as possible)
  5. more free runners from a different source, avoiding runner availability issues on Azure DevOps
  6. builds without dpc backend for testing robustness of the non-SYCLqueue/dummySYCLqueue backend
  7. preparations for integrations for public CI coverage checking
  8. fixes to yet-unseen test case failures (which will occur in 2024.6, uncovered in the new CI)
  9. fixes to conda-specific behaviors which occur in onedal.__init__ and daal4py.__init__
  10. fixes generalization problems in various CI scripts (from being conda-focused)

Regressions:

  • Additional tests had to be deselected due to the visual studio build (6 LocalOutlierFactor tests)
  • pytest downgrade for python 3.10 for windows

Note this PR removes github runner introduced in #1844 because the github.token secret has sufficient permissions (read) able to access artifacts from oneDAL (and other open-source github repos).

Current Setup tests the following for Windows and Linux (matching current CI for verification):

Python Sklearn DPCTL
3.9 1.1 yes
3.10 1.2 no
3.11 1.3 yes

Further development would be required to change these due to the fragility of some of the ci setup scripts and related pandas/scipy/numpy versions (e.g. some work will be required after new values are determined).

@icfaust icfaust changed the title Dev/GitHub action test [CI, enhancement] add sklearnex CI job with nightly oneDAL DPCPP build in github actions May 24, 2024
@samir-nasibli samir-nasibli self-requested a review May 28, 2024 06:12
@icfaust icfaust marked this pull request as draft May 28, 2024 20:31
@icfaust
Copy link
Contributor Author

icfaust commented Jul 23, 2024

/azp run CI

@icfaust
Copy link
Contributor Author

icfaust commented Aug 9, 2024

I'm running into issues with the necessary secret. This PR is on hold until this has been updated.

@icfaust
Copy link
Contributor Author

icfaust commented Aug 27, 2024

/intelci: run

@icfaust icfaust requested a review from samir-nasibli August 28, 2024 04:45
@icfaust
Copy link
Contributor Author

icfaust commented Aug 28, 2024

Issues with secrets have been resolved, and the use of a PAT is not necessary in order to access oneDAL's artifacts.

.ci/pipeline/ci.yml Outdated Show resolved Hide resolved
.ci/pipeline/ci.yml Outdated Show resolved Hide resolved
.ci/scripts/get_compatible_scipy_version.py Outdated Show resolved Hide resolved
.github/scripts/activate_components.bat Outdated Show resolved Hide resolved
.github/workflows/ci.yml Show resolved Hide resolved
.github/workflows/ci.yml Outdated Show resolved Hide resolved
deselected_tests.yaml Outdated Show resolved Hide resolved
onedal/__init__.py Show resolved Hide resolved
@icfaust icfaust requested a review from Alexsandruss August 28, 2024 22:20
@icfaust
Copy link
Contributor Author

icfaust commented Aug 28, 2024

/intelci: run

Comment on lines -1 to 3
pytest==7.4.4 ; python_version <= '3.9'
pytest==8.3.2 ; python_version >= '3.10'
pytest==7.4.4 ; python_version <= '3.10'
pytest==8.3.2 ; python_version >= '3.11'
numpy>=1.19.5 ; python_version <= '3.9'
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this needed?

Comment on lines +23 to +30
# This is necessary to prevent import errors from sklearnex and by extension
# sklearn caused by initial scipy and/or numpy versions that are installed.
# It is written in a way to be `packaging` independent. This branch occurs
# when a sklearn version is given to the script externally.
def sklearn_check_version(ver):
ver = [int(i) if i != "" else 0 for i in ver.split(".")[:2]]
sk_ver = [int(i) if i != "" else 0 for i in str(argv[1]).split(".")[:2]]
return sk_ver[0] > ver[0] or (sk_ver[0] == ver[0] and sk_ver[1] >= ver[1])
Copy link
Contributor

Choose a reason for hiding this comment

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

To me this is not obvious. In which case this should work better?

@icfaust icfaust merged commit ba7e146 into uxlfoundation:main Aug 29, 2024
23 of 24 checks passed
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.

4 participants