Skip to content
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

[FEA]: Cleanup Existing MG Tests #4187

Closed
2 tasks done
nv-rliu opened this issue Feb 22, 2024 · 0 comments
Closed
2 tasks done

[FEA]: Cleanup Existing MG Tests #4187

nv-rliu opened this issue Feb 22, 2024 · 0 comments
Assignees
Labels
feature request New feature or request improvement Improvement / enhancement to an existing function python

Comments

@nv-rliu
Copy link
Contributor

nv-rliu commented Feb 22, 2024

Is this a new feature, an improvement, or a change to existing functionality?

Improvement

How would you describe the priority of this feature request

Medium

Please provide a clear description of problem this feature solves

A lot of our existing Python MG algorithm tests are cluttered with old ways of creating edge lists and graphs. We also use a lot of nested pytest fixtures in-place of using the @pytest.parametrize feature, which could improve clarity and reduce the amount of code needed for the same behavior.

Example:

# Outdated code
datasets = utils.DATASETS_UNDIRECTED + [
    utils.RAPIDS_DATASET_ROOT_DIR_PATH / "email-Eu-core.csv"
]

G = utils.generate_cugraph_graph_from_file(
        input_data_path, directed=directed, edgevals=True
    )

ddf = dask_cudf.read_csv(
        input_data_path,
        chunksize=chunksize,
        delimiter=" ",
        names=["src", "dst", "value"],
        dtype=["int32", "int32", "float32"],
    )

dg = cugraph.Graph(directed=directed)
dg.from_dask_cudf_edgelist(
        ddf,
        source="src",
        destination="dst",
        edge_attr="value",
        renumber=True,
        store_transposed=True,
    )

@pytest.fixture(scope="module")
def input_expected_output(input_combo):
   ...

And as a result, the tests also depend on helper functions that could be cleaned up and simplified to pytest fixtures.

Describe your ideal solution

The MG tests can be cleaned up using the cugraph.datasets API, which also now supports creation of dask_cudf edge lists.
We can also use @pytest.mark.parametrize in place of input_expected_output

Describe any alternatives you have considered

No response

Additional context

No response

Code of Conduct

  • I agree to follow cuGraph's Code of Conduct
  • I have searched the open feature requests and have found no duplicates for this feature request
@nv-rliu nv-rliu added feature request New feature or request improvement Improvement / enhancement to an existing function python labels Feb 22, 2024
@nv-rliu nv-rliu self-assigned this Feb 22, 2024
@nv-rliu nv-rliu added this to the 24.04 milestone Feb 22, 2024
@nv-rliu nv-rliu changed the title [FEA]: Refactor Existing MG Tests [FEA]: Cleanup Existing MG Tests Feb 22, 2024
@nv-rliu nv-rliu removed this from the 24.04 milestone Mar 8, 2024
rapids-bot bot pushed a commit that referenced this issue Mar 14, 2024
Addresses #4187 

This PR makes improvements to old testing conventions used in `cugraph.centrality` MG tests

Instead of using nested fixtures. eg. `input_expected_output -> input_combo -> fixture_params` which can be confusing, the `@pytest.mark.parametrize` marker is used to iterate through combinations of parameters used for testing. The fixtures are also replaced by functions used to create SG and MG graphs.

Authors:
  - Ralph Liu (https://github.com/nv-rliu)

Approvers:
  - Don Acosta (https://github.com/acostadon)
  - Rick Ratzel (https://github.com/rlratzel)
  - Joseph Nke (https://github.com/jnke2016)

URL: #4197
rapids-bot bot pushed a commit that referenced this issue May 14, 2024
Addresses #4187 

This PR makes improvements to old MG testing conventions used in these testing directories:
 - centrality
 - comms
 - community
 - components
 - core
 - internals

The goals of this PR is to improve readability and use of the MG tests. Changes are similar to #4197 
> Instead of using nested fixtures. eg. `input_expected_output -> input_combo -> fixture_params` which can be confusing, the `@pytest.mark.parametrize` marker is used to iterate through combinations of parameters used for testing. The fixtures are also replaced by functions used to create SG and MG graphs.

By using the Datasets API (which now supports MG Graphs thanks to @huiyuxie), the number of imports and `testing.utils` functions can be significantly reduced.

Authors:
  - Ralph Liu (https://github.com/nv-rliu)

Approvers:
  - Rick Ratzel (https://github.com/rlratzel)

URL: #4244
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request improvement Improvement / enhancement to an existing function python
Projects
None yet
Development

No branches or pull requests

2 participants