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

Add a new notebook for SNMG benchmark runs #4091

Merged
merged 15 commits into from
Feb 13, 2024

Conversation

naimnv
Copy link
Contributor

@naimnv naimnv commented Jan 13, 2024

This PR adds new notebook for SNMG benchmark runs. It

  • simplifies bookkeeping and data presentation.
  • update each cugraph function wrappers to handle both SG and MG calls depending on a flag. The object is to have a common set of function wrappers for SG, SNMG and MNMG and put them in a common module to avoid duplication.

@naimnv naimnv requested a review from a team as a code owner January 13, 2024 01:10
Copy link

copy-pr-bot bot commented Jan 13, 2024

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@naimnv naimnv marked this pull request as draft January 13, 2024 01:10
@naimnv naimnv requested review from bdice, jnke2016 and BradReesWork and removed request for bdice January 17, 2024 18:20
@naimnv naimnv assigned naimnv and unassigned BradReesWork and nv-rliu Jan 17, 2024
@naimnv naimnv added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Jan 17, 2024
@naimnv naimnv marked this pull request as ready for review January 17, 2024 18:22
@naimnv naimnv changed the title Update notebook to include example MG runs Update notebook to include example MG benchmark Jan 17, 2024
@naimnv naimnv changed the title Update notebook to include example MG benchmark Update notebook to include MG benchmark Jan 17, 2024
@BradReesWork BradReesWork added this to the 24.02 milestone Jan 23, 2024
Copy link
Contributor

@acostadon acostadon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great to me.
just need to add your changes to the notebook credits and add 2024 to the copyrights at the bottom,
I ran it on my workstation up to scale 24 on a 2-GPU workstation and it ran perfectly.
Running on scale 25 now.

@BradReesWork
Copy link
Member

BradReesWork commented Jan 30, 2024

You need to create a new Notebook and not just add MG to this notebook. The issue is that the notebook compare against NetworkX and Nx does not support DASK or any distributed data. You would need to update the code to always collect that data back to a single host.

This will not work in MG
gdf = generate_data(v)
pdf = gdf.to_pandas()

dask_cudf does not have a "to_pandas" function

Copy link
Member

@BradReesWork BradReesWork left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs a new Notebook for MG

@BradReesWork BradReesWork added the DO NOT MERGE Hold off on merging; see PR for details label Jan 30, 2024
@BradReesWork BradReesWork changed the base branch from branch-24.02 to branch-24.04 January 30, 2024 21:35
@BradReesWork BradReesWork modified the milestones: 24.02, 24.04 Jan 30, 2024
@naimnv
Copy link
Contributor Author

naimnv commented Jan 30, 2024

Sure. We decided not to merge it last week.

@naimnv
Copy link
Contributor Author

naimnv commented Jan 30, 2024

This will not work in MG
gdf = generate_data(v)
pdf = gdf.to_pandas()

dask_cudf does not have a "to_pandas" function

@BradReesWork Just to let you know, the updated notebook's generate_data has mg=Flase by default.
def generate_data(scale, edgefactor=16, mg=False):

From correctness point of view, this code will always work.

But I agree that we need to collect the benchmark graphs back to single host and run Nx algorithms on them.

@naimnv
Copy link
Contributor Author

naimnv commented Jan 31, 2024

Early next week, I can work on this again to make a Notebook for MG.

@naimnv
Copy link
Contributor Author

naimnv commented Feb 9, 2024

@BradReesWork @acostadon
This PR has been updated to include a new notebook for SNMG benchmark runs. I will go over it again, but early review is always valuable.

@naimnv naimnv changed the title Update notebook to include MG benchmark Add new notebook for SNMG benchmark runs Feb 9, 2024
@naimnv naimnv changed the title Add new notebook for SNMG benchmark runs Add a new notebook for SNMG benchmark runs Feb 9, 2024
@naimnv naimnv removed the DO NOT MERGE Hold off on merging; see PR for details label Feb 9, 2024
@BradReesWork
Copy link
Member

/ok to test

@naimnv
Copy link
Contributor Author

naimnv commented Feb 9, 2024

/ok to test

@naimnv
Copy link
Contributor Author

naimnv commented Feb 12, 2024

/ok to test

@naimnv
Copy link
Contributor Author

naimnv commented Feb 13, 2024

/ok to test

@BradReesWork
Copy link
Member

/merge

@rapids-bot rapids-bot bot merged commit b93e114 into rapidsai:branch-24.04 Feb 13, 2024
110 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Improvement / enhancement to an existing function non-breaking Non-breaking change python
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants