Logging Refactor - Remove Print Statements by basilwong · Pull Request #139782 · pytorch/pytorch

basilwong · 2024-11-05T18:54:00Z

Summary:
Removes print statements and implements logging via the logging library.

Hopefully this will allow more control on the level of logging when running models.

Test Plan:

AOT_PARTITIONER_DEBUG=1 buck2 run @mode/opt //aps_models/ads/icvr:icvr_launcher -- mode=local_fb_fm_v4 launcher.num_workers=2

Resulting output paste: P1674535630

Full logs paste: P1674535621

pastry P1674535621 | grep "functorch/partitioners.py" | pastry

Logging results: P1674549514

Differential Revision: D61678215

pytorch-bot · 2024-11-05T18:54:04Z

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/139782

📄 Preview Python docs built from this PR
📄 Preview C++ docs built from this PR
❓ Need help or want to give feedback on the CI? Visit the bot commands wiki or our office hours

Note: Links to docs will display an error until the docs builds have been completed.

✅ No Failures

As of commit 91b30fe with merge base 3e82b1f ():
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

facebook-github-bot · 2024-11-05T18:54:12Z

This pull request was exported from Phabricator. Differential Revision: D61678215

torch/_functorch/partitioners.py

Skylion007 · 2024-11-05T19:06:14Z

torch/_functorch/partitioners.py

        )
-        sorted_sizes = sorted([(_size_of(i), str(i)) for i in saved_values])
+        logger.debug(
+            f"Theoretical Per Activation Storage Sizes: {sorted_sizes}"


Ruff will complain about this in a second

Suggested change

f"Theoretical Per Activation Storage Sizes: {sorted_sizes}"

"Theoretical Per Activation Storage Sizes: %s", {sorted_sizes}

zeshengzong · 2024-11-06T09:44:29Z

torch/_functorch/partitioners.py

        )
-        print(
+        logger.debug(
            "Count of Ops Rematerialized: ",


Suggested change

"Count of Ops Rematerialized: ",

"Count of Ops Rematerialized: %s",

Skylion007 · 2024-11-06T16:10:06Z

Check the ruff errors in lintrunner, it will complain for you for a lot of this logging.

Skylion007 · 2024-11-06T16:10:41Z

torch/_functorch/partitioners.py

        if node.op == "call_function":
            cnt[node.target.__name__] += 1
-    print(sorted(cnt.items(), key=lambda x: x[1], reverse=True))
+    logger.info(sorted(cnt.items(), key=lambda x: x[1], reverse=True))


Suggested change

logger.info(sorted(cnt.items(), key=lambda x: x[1], reverse=True))

logger.info("%s", sorted(cnt.items(), key=lambda x: x[1], reverse=True)))

facebook-github-bot · 2024-11-06T23:44:03Z

This pull request was exported from Phabricator. Differential Revision: D61678215

facebook-github-bot · 2024-11-07T01:57:14Z

This pull request was exported from Phabricator. Differential Revision: D61678215

basilwong · 2024-11-07T01:57:42Z

Updated the diff based on comments. Testing also confirms the output.

facebook-github-bot · 2024-11-07T02:01:03Z

This pull request was exported from Phabricator. Differential Revision: D61678215

basilwong · 2024-11-07T02:19:35Z

@pytorchbot label "topic: not user facing"

facebook-github-bot · 2024-11-07T09:31:31Z

This pull request was exported from Phabricator. Differential Revision: D61678215

facebook-github-bot · 2024-11-07T09:34:19Z

This pull request was exported from Phabricator. Differential Revision: D61678215

linux-foundation-easycla · 2024-11-07T09:34:23Z

The committers listed above are authorized under a signed CLA.

✅ login: basilwong / name: Basil Wong (91b30fe)

basilwong · 2024-11-07T09:40:40Z

/easycla

huydhn · 2024-11-12T17:15:34Z

@pytorchbot rebase

pytorchmergebot · 2024-11-12T17:17:17Z

@pytorchbot started a rebase job onto refs/remotes/origin/viable/strict. Check the current status here

pytorchmergebot · 2024-11-12T17:17:19Z

Successfully rebased export-D61678215 onto refs/remotes/origin/viable/strict, please pull locally before adding more changes (for example, via git checkout export-D61678215 && git pull --rebase)

facebook-github-bot · 2024-11-12T18:04:03Z

This pull request was exported from Phabricator. Differential Revision: D61678215

Summary: Removes print statements and implements logging via the logging library. Hopefully this will allow more control on the level of logging when running models. Test Plan: ``` AOT_PARTITIONER_DEBUG=1 buck2 run mode/opt //aps_models/ads/icvr:icvr_launcher -- mode=local_fb_fm_v4 launcher.num_workers=2 ``` Resulting output paste: P1674535630 * Full logs paste: P1674535621 ``` pastry P1674535621 | grep "functorch/partitioners.py" | pastry ``` Logging results: P1674549514 Reviewed By: paryxyt Differential Revision: D61678215

facebook-github-bot · 2024-11-12T18:17:45Z

This pull request was exported from Phabricator. Differential Revision: D61678215

facebook-github-bot · 2024-11-12T23:12:00Z

This pull request was exported from Phabricator. Differential Revision: D61678215

Summary: Removes print statements and implements logging via the logging library. Hopefully this will allow more control on the level of logging when running models. Test Plan: ``` AOT_PARTITIONER_DEBUG=1 buck2 run mode/opt //aps_models/ads/icvr:icvr_launcher -- mode=local_fb_fm_v4 launcher.num_workers=2 ``` Resulting output paste: P1674535630 * Full logs paste: P1674535621 ``` pastry P1674535621 | grep "functorch/partitioners.py" | pastry ``` Logging results: P1674549514 Reviewed By: paryxyt Differential Revision: D61678215

facebook-github-bot · 2024-11-12T23:57:32Z

This pull request was exported from Phabricator. Differential Revision: D61678215

facebook-github-bot · 2024-11-13T23:01:15Z

@pytorchbot merge

(Initiating merge automatically since Phabricator Diff has merged)

pytorchmergebot · 2024-11-13T23:03:38Z

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging

Check the merge workflow status
here

Summary: Removes print statements and implements logging via the logging library. Hopefully this will allow more control on the level of logging when running models. Test Plan: ``` AOT_PARTITIONER_DEBUG=1 buck2 run @mode/opt //aps_models/ads/icvr:icvr_launcher -- mode=local_fb_fm_v4 launcher.num_workers=2 ``` Resulting output paste: P1674535630 * Full logs paste: P1674535621 ``` pastry P1674535621 | grep "functorch/partitioners.py" | pastry ``` Logging results: P1674549514 Differential Revision: D61678215 Pull Request resolved: pytorch#139782 Approved by: https://github.com/paryxyt, https://github.com/jansel

pytorch-bot bot added the ciflow/inductor label Nov 5, 2024

facebook-github-bot added the fb-exported label Nov 5, 2024

Skylion007 reviewed Nov 5, 2024

View reviewed changes

torch/_functorch/partitioners.py Outdated Show resolved Hide resolved

Skylion007 reviewed Nov 5, 2024

View reviewed changes

torch/_functorch/partitioners.py Outdated Show resolved Hide resolved

Skylion007 reviewed Nov 5, 2024

View reviewed changes

zeshengzong reviewed Nov 6, 2024

View reviewed changes

Skylion007 reviewed Nov 6, 2024

View reviewed changes

basilwong force-pushed the export-D61678215 branch from 74a1275 to 71de1ef Compare November 6, 2024 23:43

basilwong force-pushed the export-D61678215 branch from 71de1ef to 35c848c Compare November 7, 2024 01:57

basilwong force-pushed the export-D61678215 branch from 35c848c to ef52d8d Compare November 7, 2024 02:00

pytorch-bot bot added the topic: not user facing topic category label Nov 7, 2024

basilwong force-pushed the export-D61678215 branch from ef52d8d to 56053d1 Compare November 7, 2024 09:31

basilwong force-pushed the export-D61678215 branch from 56053d1 to ef57d09 Compare November 7, 2024 09:34

pytorchmergebot force-pushed the export-D61678215 branch from 8fc228a to 93d720c Compare November 12, 2024 17:17

huydhn requested a review from zou3519 November 12, 2024 17:20

basilwong force-pushed the export-D61678215 branch from 93d720c to 16d0390 Compare November 12, 2024 18:03

basilwong force-pushed the export-D61678215 branch from 16d0390 to 23fafe2 Compare November 12, 2024 18:17

basilwong force-pushed the export-D61678215 branch from 23fafe2 to 04750b2 Compare November 12, 2024 23:11

basilwong force-pushed the export-D61678215 branch from 04750b2 to 91b30fe Compare November 12, 2024 23:57

jansel approved these changes Nov 13, 2024

View reviewed changes

pytorchmergebot added the merging label Nov 13, 2024

pytorchmergebot added the Merged label Nov 13, 2024

pytorchmergebot closed this in 2f1dbfe Nov 13, 2024

pytorchmergebot removed the merging label Nov 13, 2024

	f"Theoretical Per Activation Storage Sizes: {sorted_sizes}"
	"Theoretical Per Activation Storage Sizes: %s", {sorted_sizes}

	"Count of Ops Rematerialized: ",
	"Count of Ops Rematerialized: %s",

	logger.info(sorted(cnt.items(), key=lambda x: x[1], reverse=True))
	logger.info("%s", sorted(cnt.items(), key=lambda x: x[1], reverse=True)))

Conversation

basilwong commented Nov 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pytorch-bot bot commented Nov 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/139782

✅ No Failures

Uh oh!

facebook-github-bot commented Nov 5, 2024

Uh oh!

Uh oh!

Uh oh!

Skylion007 Nov 5, 2024

Choose a reason for hiding this comment

Uh oh!

Skylion007 Nov 6, 2024

Choose a reason for hiding this comment

Uh oh!

zeshengzong Nov 6, 2024

Choose a reason for hiding this comment

Uh oh!

Skylion007 commented Nov 6, 2024

Uh oh!

Skylion007 Nov 6, 2024

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot commented Nov 6, 2024

Uh oh!

facebook-github-bot commented Nov 7, 2024

Uh oh!

basilwong commented Nov 7, 2024

Uh oh!

facebook-github-bot commented Nov 7, 2024

Uh oh!

basilwong commented Nov 7, 2024

Uh oh!

facebook-github-bot commented Nov 7, 2024

Uh oh!

facebook-github-bot commented Nov 7, 2024

Uh oh!

linux-foundation-easycla bot commented Nov 7, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

basilwong commented Nov 7, 2024

Uh oh!

huydhn commented Nov 12, 2024

Uh oh!

pytorchmergebot commented Nov 12, 2024

Uh oh!

pytorchmergebot commented Nov 12, 2024

Uh oh!

facebook-github-bot commented Nov 12, 2024

Uh oh!

facebook-github-bot commented Nov 12, 2024

Uh oh!

facebook-github-bot commented Nov 12, 2024

Uh oh!

facebook-github-bot commented Nov 12, 2024

Uh oh!

facebook-github-bot commented Nov 13, 2024

Uh oh!

pytorchmergebot commented Nov 13, 2024

Merge started

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

basilwong commented Nov 5, 2024 •

edited

Loading

pytorch-bot bot commented Nov 5, 2024 •

edited

Loading

linux-foundation-easycla bot commented Nov 7, 2024 •

edited

Loading