Skip to content

fix: Use last known good oneDAL Windows Nightly for CI #2463

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

Closed

Conversation

ahuber21
Copy link
Contributor

@ahuber21 ahuber21 commented May 5, 2025

While investigating the Windows CI fails, likely introduced by uxlfoundation/oneDAL#3155, use a last known good commit hash for checks in this repository.

Copy link

codecov bot commented May 5, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Flag Coverage Δ
azure 79.87% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ahuber21 ahuber21 changed the title [do not merge] debug WinNightly fail fix: Use last known good oneDAL Windows Nightly for CI May 8, 2025
@ahuber21 ahuber21 added bug Something isn't working infra testing Tests for sklearnex/daal4py/onedal4py & patching sklearn labels May 8, 2025
@icfaust
Copy link
Contributor

icfaust commented May 8, 2025

This is neither a fix, or nor a nightly now. It breaks the utility of the CI in providing the latest changes for testing in sklearnex publicly. I believe reverting the known oneDAL PR is the better course of action. i would think this would be the proper solution if we had evidence of which symbol is missing. did the dependency walker yield anything @ahuber21 ?

@ahuber21
Copy link
Contributor Author

ahuber21 commented May 8, 2025

Dependency Walker didn't help. Still convinced that it's a tooling issue. If our tools prevent us from pushing updates, I think we need to fix or disable them. It is just a temporary fix, but IMO reverting 3155 is not an option.

@icfaust
Copy link
Contributor

icfaust commented May 8, 2025

Gotcha. Then the solution should be disabling the windows side of the workflow entirely with a single line if statement. As the person who developed this CI, I dont see a compelling reason to keep the windows workflow running given that it now just duplicates the azp windows jobs on sklearn versions we generally dont officially support. it will waste of our finite github resources, but I am willing to listen to technical arguments that you otherwise may have @ahuber21 @ethanglaser @napetrov

@ethanglaser
Copy link
Contributor

Should this be closed at this point?

@ahuber21 ahuber21 closed this Jun 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working infra testing Tests for sklearnex/daal4py/onedal4py & patching sklearn
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants