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 environment variable to log cudf.pandas fallback calls #16161

Open
wants to merge 7 commits into
base: branch-24.08
Choose a base branch
from

Conversation

mroeschke
Copy link
Contributor

Description

Introduces a new environment variable CUDF_PANDAS_SLOW_LOG which will create a structured log of the call that failed.

An example of the log is

INFO:root:{"debug_type": "CUDF_PANDAS_SLOW_LOG", "failed_call": "pandas._libs.interval.Interval(0,1,)", "exception": "Exception", "exception_message": "Cannot transform _Unusable", "pandas_object": "pandas._libs.interval.Interval", "passed_args": "0,1,", "passed_kwargs": {}}

I could turn this into a warning instead, but I imagine we would want to first utilize this to parse the failures and see generalized failures in aggregate

Checklist

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

@mroeschke mroeschke added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change cudf.pandas Issues specific to cudf.pandas labels Jul 2, 2024
@mroeschke mroeschke requested a review from a team as a code owner July 2, 2024 02:24
@mroeschke mroeschke requested review from wence- and bdice July 2, 2024 02:24
@github-actions github-actions bot added the Python Affects Python cuDF API. label Jul 2, 2024
Copy link
Contributor

@wence- wence- left a comment

Choose a reason for hiding this comment

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

I think it would be nice to move all the formatting logic into the _logger file.

@@ -0,0 +1,26 @@
# SPDX-FileCopyrightText: Copyright (c) 2023-2024, NVIDIA CORPORATION & AFFILIATES.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# SPDX-FileCopyrightText: Copyright (c) 2023-2024, NVIDIA CORPORATION & AFFILIATES.
# SPDX-FileCopyrightText: Copyright (c) 2024, NVIDIA CORPORATION & AFFILIATES.

Comment on lines +941 to +978
from ._logger import StructuredMessage, logger

def reprify(arg) -> str:
try:
return repr(arg)
except Exception:
return "<REPR FAILED>"

module = getattr(slow_args[0], "__module__", "")
obj_name = getattr(
slow_args[0], "__name__", type(slow_args[0]).__name__
)
slow_object = f"{module}.{obj_name}"
# TODO: maybe use inspect.signature to map called args and kwargs
# to their keyword names
called_args = ",".join((reprify(val) for val in slow_args[1]))
if len(slow_args) == 3:
fmt_kwargs = ",".join(
f"{kwarg}={reprify(value)}"
for kwarg, value in slow_args[2].items()
)
called_args = ",".join((called_args, fmt_kwargs))
passed_kwargs = {
kwarg: reprify(value)
for kwarg, value in slow_args[2].items()
}
else:
passed_kwargs = {}
message = StructuredMessage(
"CUDF_PANDAS_SLOW_LOG",
failed_call=f"{slow_object}({called_args})",
exception=type(err).__name__,
exception_message=str(err),
pandas_object=slow_object,
passed_args=called_args,
passed_kwargs=passed_kwargs,
)
logger.info(message)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we pull all of this logic into the logger, so that we just do:

if should_log:
    from ._logger import log_fallback
    log_fallback(slow_args, slow_kwargs)

Comment on lines +957 to +968
if len(slow_args) == 3:
fmt_kwargs = ",".join(
f"{kwarg}={reprify(value)}"
for kwarg, value in slow_args[2].items()
)
called_args = ",".join((called_args, fmt_kwargs))
passed_kwargs = {
kwarg: reprify(value)
for kwarg, value in slow_args[2].items()
}
else:
passed_kwargs = {}
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are we never looking slow_kwargs?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think slow_kwargs is always empty. Because kwargs is getting subsumed by args in the _fast_slow_function_call calls. I opened #16266 to fix.

@Matt711
Copy link
Contributor

Matt711 commented Jul 11, 2024

Just an fyi, I don't see the logs anywhere when I set the environment variable and run the pandas unit tests with cudf.pandas. Similar problem I've had in #15910

@mroeschke
Copy link
Contributor Author

Just an fyi, I don't see the logs anywhere when I set the environment variable and run the pandas unit tests with cudf.pandas. Similar problem I've had in #15910

The logs should show up in the cloned pandas-testing/pandas-test directory when running with CUDF_PANDAS_SLOW_LOG=true. Did you happen to run this with a non-editable install of cudf?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Status: In Progress
Development

Successfully merging this pull request may close these issues.

None yet

4 participants