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

Switch pytest traceback to native #1389

Merged
merged 2 commits into from
Nov 1, 2024
Merged

Conversation

galipremsagar
Copy link
Contributor

Description

In cudf & cuml we have observed a ~10% to ~20% respectively speed up of pytest suite execution by switching pytest traceback to --native:

currently:

102474 passed, 2117 skipped, 902 xfailed in 892.16s (0:14:52)

--tb=short:

102474 passed, 2117 skipped, 902 xfailed in 898.99s (0:14:58)

--tb=no:

102474 passed, 2117 skipped, 902 xfailed in 815.98s (0:13:35)

--tb=native:

102474 passed, 2117 skipped, 902 xfailed in 820.92s (0:13:40)

This PR makes similar change to dask-cuda repo.

xref: rapidsai/cudf#16851

Checklist

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

@galipremsagar galipremsagar added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Sep 25, 2024
@galipremsagar galipremsagar self-assigned this Sep 25, 2024
@galipremsagar galipremsagar requested a review from a team as a code owner September 25, 2024 15:24
@github-actions github-actions bot added the python python code needed label Sep 25, 2024
@pentschev
Copy link
Member

pentschev commented Sep 25, 2024

Do you have more information exactly how does this compare to --tb=auto? From the docs:

pytest --tb=auto    # (default) 'long' tracebacks for the first and last
                     # entry, but 'short' style for the other entries
pytest --tb=long    # exhaustive, informative traceback formatting
pytest --tb=short   # shorter traceback format

It seems that with auto we get "exhaustive" traceback (not really sure what that means), is that the case with native too? Although 11.8.0 builds seem faster, 12.5.1 do not (see table below), still waiting on 12.0.1 to complete, but in general I'm not sure there's conclusive benefit here to justify this change, in particular if it happens to omit useful parts of the traceback.

CUDA version This change Build PR #1385 Build PR #1384
11.8.0 57:11 63:28 65:07
12.0.1 48:25 47:58 49:51
12.5.1 N/A 74:39 84:57

EDIT: I had 12.0.1/12.5.1 results swapped, I've corrected that now.

Copy link
Member

@pentschev pentschev left a comment

Choose a reason for hiding this comment

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

Based on the uncertainty from this PR as per my comment, as well as the seeming regression in performance in UCX-Py, I'm "requesting changes" until we can determine what's the correct way forward.

Please post any other thoughts and comments in the PR in the meantime.

@pentschev pentschev changed the base branch from branch-24.10 to branch-24.12 October 28, 2024 19:32
@pentschev
Copy link
Member

/merge

@rapids-bot rapids-bot bot merged commit fc80d43 into rapidsai:branch-24.12 Nov 1, 2024
28 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 python code needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants