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.

Sign up for GitHub

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

Jump to bottom

Generate GPU vs CPU usage metrics per pytest file in pandas testsuite for cudf.pandas #16739

Merged
merged 31 commits into from
Sep 19, 2024
Merged

Generate GPU vs CPU usage metrics per pytest file in pandas testsuite for cudf.pandas #16739

merged 31 commits into from
Sep 19, 2024

Conversation

galipremsagar
Copy link
Contributor

@galipremsagar galipremsagar commented Sep 4, 2024 •
edited
Loading

Description

This PR introduces GPU and CPU usage reporting to cudf.pandas pytest suite and the generated metrics will be available for viewing in the existing pandas pytest summary page:
https://github.com/rapidsai/cudf/actions/runs/10886370333/attempts/1#summary-30220192117

Screenshot 2024-09-16 at 2 39 07 PM

Note: I'm aware of cases of where both GPU and CPU usage show 0%, which is due to various reasons that I'm working on addressing in a follow-up PR.

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@github-actions github-actions bot added Python Affects Python cuDF API. cudf.pandas Issues specific to cudf.pandas labels Sep 4, 2024
@galipremsagar galipremsagar added non-breaking Non-breaking change improvement Improvement / enhancement to an existing function labels Sep 9, 2024
@galipremsagar galipremsagar changed the title test pr Generate GPU vs CPU usage metrics per pytest file in pandas testsuite for cudf.pandas Sep 16, 2024
@galipremsagar galipremsagar marked this pull request as ready for review September 16, 2024 19:43
@galipremsagar galipremsagar requested review from a team as code owners September 16, 2024 19:43
@galipremsagar galipremsagar mentioned this pull request Sep 16, 2024
Closed
3 tasks
@galipremsagar galipremsagar self-assigned this Sep 16, 2024
galipremsagar
galipremsagar commented Sep 16, 2024
View reviewed changes
Copy link
Contributor Author

@galipremsagar galipremsagar left a comment

Choose a reason for hiding this comment

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

Note: I'm aware of cases of where both GPU and CPU usage shows 0%, which is due to various reasons that I'm working on addressing in a follow-up PR.

Matt711
Matt711 reviewed Sep 16, 2024
View reviewed changes
Copy link
Contributor

@Matt711 Matt711 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I noticed that some of the tests have 0% reported for both CPU and GPU usage. I guess we aren't catching some of the dummy function calls (eg. pr_df['_slow_function_call']=0?

Edit: My apologies, you're aware of this already.

mroeschke
mroeschke reviewed Sep 17, 2024
View reviewed changes
ci/cudf_pandas_scripts/pandas-tests/job-summary.py Outdated
pr_df['CPU Usage'] = pr_df['CPU Usage'].astype(str) + '%'
pr_df['GPU Usage'] = pr_df['GPU Usage'].astype(str) + '%'

pr_df['CPU Usage'] = pr_df['CPU Usage'].replace('nan%', '0%')
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggestion: IMO would be better to fillna(0) before the astype(str) so we don't have to do this replace

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

mroeschke
mroeschke reviewed Sep 17, 2024
View reviewed changes
ci/cudf_pandas_scripts/pandas-tests/job-summary.py Outdated
@@ -68,8 +68,20 @@ def emoji_failed(x):
pr_df = pd.DataFrame.from_dict(pr_results, orient="index").sort_index()
main_df = pd.DataFrame.from_dict(main_results, orient="index").sort_index()
diff_df = pr_df - main_df
pr_df['CPU Usage'] = ((pr_df['_slow_function_call']/(pr_df['_slow_function_call'] + pr_df['_fast_function_call']))*100.0).round(1)
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Could you put the denominator calculation on it's own line?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@galipremsagar
Copy link
Contributor Author

I noticed that some of the tests have 0% reported for both CPU and GPU usage. I guess we aren't catching some of the dummy function calls (eg. pr_df['_slow_function_call']=0?

Edit: My apologies, you're aware of this already.

@Matt711 I pushed a fix to this PR that will improve the missing reports, there would a few instances of such cases now.

Matt711
Matt711 reviewed Sep 19, 2024
View reviewed changes
Copy link
Contributor

@Matt711 Matt711 left a comment

Choose a reason for hiding this comment

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

Thanks! Everything LGTM. Before I approve, I'd like to see the final table.

python/cudf/cudf/pandas/scripts/conftest-patch.py Show resolved Hide resolved
@galipremsagar
Copy link
Contributor Author

Thanks! Everything LGTM. Before I approve, I'd like to see the final table.

Final table is here: https://github.com/rapidsai/cudf/actions/runs/10941284785/attempts/1#summary-30389636693

@galipremsagar
Copy link
Contributor Author

/merge

@rapids-bot rapids-bot bot merged commit dafb3e7 into rapidsai:branch-24.10 Sep 19, 2024
100 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@raydouglass raydouglass raydouglass approved these changes

@Matt711 Matt711 Matt711 approved these changes

@isVoid isVoid Awaiting requested review from isVoid isVoid is a code owner automatically assigned from rapidsai/cudf-python-codeowners

@charlesbluca charlesbluca Awaiting requested review from charlesbluca charlesbluca is a code owner automatically assigned from rapidsai/cudf-python-codeowners

@mroeschke mroeschke Awaiting requested review from mroeschke

Assignees

@galipremsagar galipremsagar

Labels
cudf.pandas Issues specific to cudf.pandas improvement Improvement / enhancement to an existing function non-breaking Non-breaking change Python Affects Python cuDF API.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@galipremsagar @raydouglass @mroeschke @Matt711