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 an environment variable for handling fallback in cudf.pandas #15910

Closed

Conversation

Matt711
Copy link
Contributor

@Matt711 Matt711 commented Jun 3, 2024

Description

This PR wraps up #14975 and extends PR #15837. It adds a fallback debugging mode to _fast_slow_function_call that returns warnings for different types of fallback that occur in cudf.pandas. The types of fallback covered are:

  • Out of memory errors, for the sake of planning No OOM related work
  • AttributeErrors for missing functionality
  • TypeErrors for differing function signatures

Checklist

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

Copy link

copy-pr-bot bot commented Jun 3, 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.

@github-actions github-actions bot added Python Affects Python cuDF API. cudf.pandas Issues specific to cudf.pandas labels Jun 3, 2024
@Matt711 Matt711 self-assigned this Jun 3, 2024
@Matt711 Matt711 marked this pull request as ready for review June 11, 2024 20:17
@Matt711 Matt711 requested a review from a team as a code owner June 11, 2024 20:17
@Matt711 Matt711 requested review from vyasr and bdice June 11, 2024 20:17
@mroeschke
Copy link
Contributor

mroeschke commented Jun 14, 2024

Looks pretty good. Could you show a small example of how this would look when using the run-pandas-tests.sh (maybe just run it on one test file with a few tests)?

@Matt711 Matt711 force-pushed the feature/fallback-debugging branch from 3135357 to 474f44a Compare June 14, 2024 16:02
@Matt711
Copy link
Contributor Author

Matt711 commented Jun 14, 2024

/ok to test

@Matt711
Copy link
Contributor Author

Matt711 commented Jun 14, 2024

/ok to test

@Matt711
Copy link
Contributor Author

Matt711 commented Jun 18, 2024

Looks pretty good. Could you show a small example of how this would look when using the run-pandas-tests.sh (maybe just run it on one test file with a few tests)?

I'm not seeing warnings where I expect them, which makes me think the environment variable is being set when the pandas tests are run. This is the command I'm using.
export CUDF_PANDAS_FALLBACK_DEBUGGING=True && python/cudf/cudf/pandas/scripts/run-pandas-tests.sh -n auto -v -p cudf.pandas tests/groupby/ | grep Warning

@bdice
Copy link
Contributor

bdice commented Jun 18, 2024

@Matt711 Are the warnings going to stderr? The pipe to grep will only capture stdout. You might need something like 2>&1 in there.

@Matt711
Copy link
Contributor Author

Matt711 commented Jun 20, 2024

@Matt711 Are the warnings going to stderr? The pipe to grep will only capture stdout. You might need something like 2>&1 in there.

test_groupby_agg_no_extra_calls should definitely return NotImplemented warnings, but I don't see them in stdout. I don't think the test are being run with cudf.pandas despite -p cudf.pandas being passed.

(rapids) coder ➜ ~/cudf $ pytest -v -p cudf.pandas ./pandas-testing/pandas-tests/tests/groupby/aggregate/test_aggregate.py::test_groupby_agg_no_extra_calls 2>&1
============================================================================================================================================ test session starts ============================================================================================================================================
platform linux -- Python 3.10.14, pytest-7.4.4, pluggy-1.5.0 -- /home/coder/.conda/envs/rapids/bin/python3.10
cachedir: .pytest_cache
hypothesis profile 'ci' -> deadline=None, suppress_health_check=[HealthCheck.too_slow, HealthCheck.differing_executors], database=DirectoryBasedExampleDatabase(PosixPath('/home/coder/cudf/.hypothesis/examples'))
benchmark: 4.0.0 (defaults: timer=time.perf_counter disable_gc=False min_rounds=5 min_time=0.000005 max_time=1.0 calibration_precision=10 warmup=False warmup_iterations=100000)
rootdir: /home/coder/cudf/pandas-testing/pandas-tests
configfile: pyproject.toml
plugins: anyio-4.4.0, hypothesis-6.103.2, benchmark-4.0.0, cases-3.8.5, cov-5.0.0, xdist-3.6.1
collected 1 item                                                                                                                                                                                                                                                                                            

pandas-testing/pandas-tests/tests/groupby/aggregate/test_aggregate.py::test_groupby_agg_no_extra_calls PASSED                                                                                                                                                                                         [100%]

============================================================================================================================================= 1 passed in 0.11s =============================================================================================================================================

@Matt711 Matt711 added the 0 - Blocked Cannot progress due to external reasons label Jul 8, 2024
@Matt711
Copy link
Contributor Author

Matt711 commented Jul 12, 2024

Closing this PR in favor of #16161

@Matt711 Matt711 closed this Jul 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0 - Blocked Cannot progress due to external reasons 3 - Ready for Review Ready for review by team cudf.pandas Issues specific to cudf.pandas feature request New feature or request non-breaking Non-breaking change Python Affects Python cuDF API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants