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 option to disable logger while compiling to avoid graph breaks #6496

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

ShellyNR
Copy link

@ShellyNR ShellyNR commented Sep 5, 2024

adding an option to disable calls for logger while compiling to avoid graph breaks. Here I used an environment variable to determine whether to activate this option, but it can also be determined using the json config file or any other way you see fit.

@loadams loadams requested review from tohtana and removed request for awan-10 September 5, 2024 15:40
@tohtana
Copy link
Contributor

tohtana commented Sep 5, 2024

Hi @ShellyNR, thank you for the great catch!
Can we also show a warning to recommend setting DISABLE_LOGS_WHILE_COMPILING=1 when the compiler is enabled but the variable is not set?

@tohtana tohtana self-assigned this Sep 6, 2024
@SNahir
Copy link

SNahir commented Sep 19, 2024

Hi, I added a warning as you suggested, I used a logger filter for this as it runs every time a log record is created
@tohtana

nonlocal warn
if torch.compiler.is_compiling() and not warn:
warn = True
logger.warning("It is recommended to set DISABLE_LOGS_WHILE_COMPILING=1,"
Copy link
Contributor

Choose a reason for hiding this comment

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

Rephrase the warning to reflect that it applies to compile-mode execution and control by env var. For example,

Suggested change
logger.warning("It is recommended to set DISABLE_LOGS_WHILE_COMPILING=1,"
logger.warning("To avoid graph breaks in compile-mode, it is recommended to disable logging by setting env var DISABLE_LOGS_WHILE_COMPILING=1,"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants