scikit-learn-intelex
scikit-learn-intelex copied to clipboard
intel
TEST: test coverage for sklearnex SPMD ifaces
Description
Scope:
- Introduces initial validation of spmd algos via mpi-pytest
- Identify and create follow-up tasks for issues identified in initial testing
For each sklearnex spmd algorithm, individual manually-created tests and parametrized synthetic tests validate model attributes and prediction results against batch implementations
Job with infra branch: http://intel-ci.intel.com/eefc46ad-f4dd-f157-a13b-a4bf010d0e2e
Update job with infra branch: http://intel-ci.intel.com/ef02681e-c97c-f1ba-a9d0-a4bf010d0e2e
Latest CI with infra branch: http://intel-ci.intel.com/ef341186-2255-f106-a541-a4bf010d0e2e
There was mention of using https://github.com/intel/scikit-learn-intelex/blob/main/onedal/cluster/tests/test_dbscan.py#L52 logic for dbscan synthetic data generation, but on closer look its just a basic random data generation, not using epsilon or any dbscan params. Since I have been able to generate meaningful data using make_blobs, I am going to stick with it unless there are objections.
Latest CI: http://intel-ci.intel.com/ef38b562-3e49-f1f9-894d-a4bf010d0e2e
@ethanglaser for a such great job done!
I'll take an even deeper look at all the PR later.
I would suggest this testing to be as consistent as possible with other modules in sklearnex.
All SPMD ifaces have interability with dpnp/dpctl inputs. I suggest saving this for testing. SPMD ifaces are held up
- DPNP inputs with GPU sycl_queue
- DPCTL inputs with GPU sycl_queue
- numpy inputs and queue param with GPU sycl_queue provided.
This will be extended for Array API inputs as well.
I suggest to use get_dataframes_and_queues(dataframe_filter_="dpnp,dpctl", device_filter_="gpu",), _as_numpy and _convert_to_dataframe like in other algos modules testing.
I am also working on refactoring and extending get_dataframes_and_queues for numpy inputs and sycl_queue on #1909
further integration. After #1909 it is just needed updated to get_dataframes_and_queues(dataframe_filter_="dpnp,dpctl,np_sycl", device_filter_="gpu") for enabling it for such where numpy input with queue param enabled.
I suggest to use
get_dataframes_and_queues(dataframe_filter_="dpnp,dpctl", device_filter_="gpu",),_as_numpyand_convert_to_dataframelike in other algos modules testing.
Thanks, addressed
Latest CI with infra branch: http://intel-ci.intel.com/ef3fe1d1-c22d-f1dc-b5ca-a4bf010d0e2e
It looks like all tests are skipped in both public and private CI. is it expected?
It looks like all tests are skipped in both public and private CI. is it expected?
They will only run with GPU and dpctl (private CI GPU tests on py 3.9-3.10)
Latest CI with infra branch: http://intel-ci.intel.com/ef4a161a-e33a-f1fc-b023-a4bf010d0e2e
I see also that different dtypes are not tested here. Is this parametrizing skipped on purpose?
I see also that different dtypes are not tested here. Is this parametrizing skipped on purpose?
Not on purpose - I will add it in. Good point.
Also it is not clear how you run this tests? https://pytest-mpi.readthedocs.io/en/latest/usage.html#
I think make sense to add some readme files/update docs in case how to run this tests.
Currently only added to internal CI since this is where we have GPU validation - see infra PR 712. Not sure if it makes sense to document in public repo.
- I don't see skips in test suits for the know issues, is it possible define defects by disabling related test cases?
There are pytest.skip( ) within the test functions - mostly because skips are dependent on parameters to function (except forest)
does it work when a number of processes to run the test <2? Does it make sense to add @pytest.mark.mpi(min_size=2)?
Infra sets up tests to run with 4 processes, but it would just run batch GPU if 1 is used, so I don't see why it would not work. But all tests are set up to skip if mpi or gpu are not present
Currently only added to internal CI since this is where we have GPU validation - see infra PR 712. Not sure if it makes sense to document in public repo.
We are adding a testing module to the public repository. It seems to me it makes sense to add a doc with info about how to launch, including some links to mpi-pytest is given. Could be done in this PR or in the separate PR as well. We have a check box in our PR templates for adding/updatung docs.
- I don't see skips in test suits for the know issues, is it possible define defects by disabling related test cases?
There are pytest.skip( ) within the test functions - mostly because skips are dependent on parameters to function (except forest)
Yes, I saw it before. My question raised because it is not everywhere. For example, for Kmeans I did not find the skip for an issue specified in the description.
Yes, I saw it before. My question raised because it is not everywhere. For example, for Kmeans I did not find the skip for an issue specified in the description.
True - in some cases its not handled by pytest.skip() but instead aspects of the test are commented. KMeans for instance the iter check is commented out and init check, because I want to have some validation still. If there is a good way to handle this instead with pytest.skip I am open to other ideas.
We are adding a testing module to the public repository. It seems to me it makes sense to add a doc with info about how to launch, including some links to mpi-pytest is given. Could be done in this PR or in the separate PR as well. We have a check box in our PR templates for adding/updatung docs.
I will update SPMD docs updates task to include this - we currently do not have documentation of our spmd interfaces at all so I don't know if it could be added yet.
Thanks for reviews. Still working out some CI fixes with introduction of float32 but will share job once its cleaner.
I see some test fails on the latest CI job provided. I will not dismiss my approve, but please attach the green CI link before the merge. Feel free to ask review again if required.