CI: Initial additions for fp32 and windows GPU test support #1778
Conversation
samir-nasibli
changed the title
|
@ethanglaser PRs #1674 should solve some of the outstanding issues with IncrementalEmpiricalCovariance for fp32 sklearnex testing, but it needs #1795 to be pulled in first. I will make a meeting for #1674, but could you review #1795? Its a relatively small change, already passes CI |
|
@ethanglaser please rebase your branch |
|
Latest CI with just GPU tests: http://intel-ci.intel.com/ef0e2f47-e2c3-f1f1-aee4-a4bf010d0e2e |
Co-authored-by: Samir Nasibli <[email protected]>
ethanglaser
changed the title
| @@ -66,7 +66,8 @@ def test_dense_self_rbf_kernel(queue): | |||
| result = rbf_kernel(X, queue=queue) | |||
| expected = sklearn_rbf_kernel(X) | |||
|
|
|||
| assert_allclose(result, expected, rtol=1e-14) | |||
| tol = 1e-5 if result.dtype == np.float32 else 1e-14 | |||
There was a problem hiding this comment.
Yikes, a 10^9 change in performance. Would this also warrant an investigation?
There was a problem hiding this comment.
I am not sure why 1e-14 was used here specifically - generally we don't go more specific than 1e-7. For rtol, 1e-5 should be acceptable, as this is a typical threshold used in other fp32 testing
|
Should be ready for final review |
ethanglaser
changed the title
|
/intelci: run |
|
CI with infra branch (includes fp32 + windows GPU validation): http://intel-ci.intel.com/ef2436e8-ef5e-f182-b1e2-a4bf010d0e2e |
| @@ -85,6 +85,8 @@ def test_generated_dataset(queue, dtype, n_dim, n_cluster): | |||
| d, i = nn.fit(rs_centroids).kneighbors(cs) | |||
| # We have applied 2 sigma rule once | |||
| desired_accuracy = int(0.9973 * n_cluster) | |||
| if d.dtype == np.float64: | |||
| desired_accuracy = desired_accuracy - 1 | |||
There was a problem hiding this comment.
What was the logic behind the desired accuracy -1?
There was a problem hiding this comment.
it matches a minor threshold change added to test_kmeans (https://github.com/intel/scikit-learn-intelex/blob/main/onedal/cluster/tests/test_kmeans.py#L87) not sure exactly how the desired_accuracy was set but seems the threshold doesn't necessarily matchup with actual performance
There was a problem hiding this comment.
I see, the kmeans++ init issue may get fixed with the changes proposed here uxlfoundation/oneDAL#2796. The onedal code uses 1 trial at the moment.
There was a problem hiding this comment.
looks good to me, but worth waiting for @icfaust about all the force_all_finite business
|
Thanks for reviews :) |
Description
Changes for green CI on GPU validation on devices without fp64 support (includes windows GPU sklearnex validation)
To be merged with infra PR 713
Changes include: