-
Notifications
You must be signed in to change notification settings - Fork 213
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
Add missing dependencies to compute probabilities function #2904
Add missing dependencies to compute probabilities function #2904
Conversation
Please share stable jobs for this fixes with sklearnex tests deselected |
Here sklearnex branch main branch, where this test is deselected. Please update and share the run with the scikit-learn-intelex spmd logreg test deselected. Please use icx compiler |
ed6fd47
to
469062a
Compare
rerun on rebased branch: http://intel-ci.intel.com/ef7691b7-b396-f153-808d-a4bf010d0e2e |
CI with enabled test: http://intel-ci.intel.com/ef769255-f5cf-f10a-99ec-a4bf010d0e2e |
@@ -87,7 +88,8 @@ sycl::event compute_probabilities_sparse(sycl::queue& q, | |||
const std::int64_t p = parameters.get_dimension(0) - (fit_intercept ? 1 : 0); | |||
|
|||
auto fill_event = fill<Float>(q, probabilities, Float(1), deps); | |||
Float w0 = fit_intercept ? parameters.get_slice(0, 1).at_device(q, 0l) : 0; // Poor perfomance | |||
Float w0 = | |||
fit_intercept ? parameters.get_slice(0, 1).at_device(q, 0l, deps) : 0; // Poor perfomance |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if the fill event has no deps with w0 computation looks good
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please rebase and re-run CI checks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@avolkov-intel I don't see that sklearnex branch has changes for deselecting required spmd tests for logreg. Please address comments and rebase your branches for the check
/intelci: run |
CI with enabled test: http://intel-ci.intel.com/ef79c8a7-7928-f1dd-a4c8-a4bf010d0e2d |
@avolkov-intel please provide the job with the rebased sklearnex main branch with spmd logreg test deselection + current branch |
@avolkov-intel all spmd sklearnex tests are skipped. Could you please check what it the issue? spmd/linear_model/tests/test_logistic_regression_spmd.py sssssssssssssss [ 51%]
ssssssssssssssssssssssssssssssssss |
Wrong place was looking. Looks good now. |
Description
Add missing dependency for at_device function and resolve logreg test failures