Skip certain cugraph-pyg tests when torch_sparse is not available
#3962
Conversation
tingyu66
added
non-breaking
| @@ -201,6 +201,10 @@ def test_cugraph_loader_from_disk_subset(): | |||
|
|
|||
| @pytest.mark.skipif(isinstance(torch, MissingModule), reason="torch not available") | |||
| def test_cugraph_loader_from_disk_subset_csr(): | |||
| # TODO: The CSC/CSR code path in cugraph_sampler.py requires torch_sparse | |||
| # as dependency. Remove this check when torch_sparse becomes a dependency. | |||
There was a problem hiding this comment.
torch-sparse will never be a dependency of cuGraph-PyG. CSC model is an optional feature, and torch-sparse is only a test dependency of cuGraph-PyG. Eventually, I think PyG is going to throw out torch-sparse altogether anyways.
There was a problem hiding this comment.
cugraph/python/cugraph-pyg/cugraph_pyg/sampler/cugraph_sampler.py
Lines 420 to 425 in 35875a8
IIRC, data.put_edge_index() uses SparseTensor internally from torch_sparse for CSC/CSR types, so I don't think it's just a test dependency.
There was a problem hiding this comment.
Yes, but that is optional. PyG users are not required to use put_edge_index or get_edge_index and we don't want to force torch-sparse unless they're using CSC mode, which uses those functions. In depenencies.yaml, we specify torch-sparse as a test-only dependency.
| @@ -201,6 +201,10 @@ def test_cugraph_loader_from_disk_subset(): | |||
|
|
|||
| @pytest.mark.skipif(isinstance(torch, MissingModule), reason="torch not available") | |||
| def test_cugraph_loader_from_disk_subset_csr(): | |||
| # TODO: The CSC/CSR code path in cugraph_sampler.py requires torch_sparse | |||
| # as dependency. Remove this check when torch_sparse becomes a dependency. | |||
| pytest.importorskip("torch_sparse", reason="PyTorch Sparse not available") | |||
There was a problem hiding this comment.
importorskip is not the RAPIDS-preferred way to handle skipping. This should be changed to using import_optional and pytest.mark.skipif like we do for torch.
There was a problem hiding this comment.
They have been corrected. PTAL.
| @@ -334,6 +338,8 @@ def test_cugraph_loader_e2e_coo(): | |||
| @pytest.mark.skipif(isinstance(torch, MissingModule), reason="torch not available") | |||
| @pytest.mark.parametrize("framework", ["pyg", "cugraph-ops"]) | |||
| def test_cugraph_loader_e2e_csc(framework): | |||
| pytest.importorskip("torch_sparse", reason="PyTorch Sparse not available") | |||
There was a problem hiding this comment.
Same here, we should be using import_optional and skipif.
tingyu66
changed the title
cugraph-pyg tests when torch_scatter is not availablecugraph-pyg tests when torch_sparse is not available
|
/merge |
The CSC (CSR) code path in cugraph-pyg requires
torch_sparsepackage. However,torch_sparsedoes not seem to work out of box for rockylinux8. This PR fixes such CI failures.Closes https://github.com/rapidsai/graph_dl/issues/343