Expose option to disable CRC-32 computation during `torch.save` #137735

mikaylagawarecki · 2024-10-10T21:42:52Z

Option only works in open source, not internal

Stack from ghstack (oldest at bottom):

-> Expose option to disable CRC-32 computation during torch.save #137735

Differential Revision: D64545647

[ghstack-poisoned]

pytorch-bot · 2024-10-10T21:42:55Z

🔗 Helpful Links

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

📄 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 8315e24 with merge base 53e356a ():
💚 Looks good so far! There are no failures yet. 💚

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

ghstack-source-id: 8f55cec Pull Request resolved: #137735

[ghstack-poisoned]

ghstack-source-id: c98df38 Pull Request resolved: #137735

…save`" [ghstack-poisoned]

ghstack-source-id: 6a551f4 Pull Request resolved: #137735

…save`" [ghstack-poisoned]

ghstack-source-id: b3bc417 Pull Request resolved: #137735

…save`" [ghstack-poisoned]

ghstack-source-id: 9184234 Pull Request resolved: #137735

…save`" [ghstack-poisoned]

ghstack-source-id: ef4eb95 Pull Request resolved: #137735

…save`" [ghstack-poisoned]

ghstack-source-id: 88e39a9 Pull Request resolved: #137735

albanD

Only small nits, sounds pretty good in general!

test/test_serialization.py

torch/serialization.py

albanD · 2024-10-11T19:17:26Z

torch/serialization.py

+    return _compute_crc32
+
+
+def set_default_crc32_options(compute_crc32: bool):


nit: why set_default_crc32_options vs set_crc32_options ? There is no way to override this flag from the end user of torch.save point of view.

will use set_crc32_options, the default was for consistency with the naming of other similar serialization APIs

albanD · 2024-10-11T19:18:16Z

third_party/miniz-2.1.0/miniz.h

    MZ_ZIP_FLAG_WRITE_ALLOW_READING = 0x8000,
-    MZ_ZIP_FLAG_ASCII_FILENAME = 0x10000
+    MZ_ZIP_FLAG_ASCII_FILENAME = 0x10000,
+    MZ_ZIP_FLAG_DO_NOT_COMPUTE_CRC32 = 0x20000, /* don't compute the crc32 of file data that's being added. */


I forgot what's the story with these changes and if they will work internally?

won't work internally as internal has different build options for miniz

…save`" [ghstack-poisoned]

ghstack-source-id: 661896d Pull Request resolved: #137735

…save`" Option only works in open source, not internal [ghstack-poisoned]

ghstack-source-id: fe3cc74 Pull Request resolved: #137735

mikaylagawarecki · 2024-10-17T16:39:39Z

@mikaylagawarecki has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

…save`" Option only works in open source, not internal Differential Revision: [D64545647](https://our.internmc.facebook.com/intern/diff/D64545647) [ghstack-poisoned]

ghstack-source-id: 40deb82 Pull Request resolved: #137735

mikaylagawarecki · 2024-10-17T18:34:59Z

@mikaylagawarecki has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

…save`" Option only works in open source, not internal Differential Revision: [D64545647](https://our.internmc.facebook.com/intern/diff/D64545647) [ghstack-poisoned]

Pull Request resolved: #137735 Option only works in open source, not internal @imported-using-ghimport Differential Revision: [D64545647](https://our.internmc.facebook.com/intern/diff/D64545647/) ghstack-source-id: 249920368

facebook-github-bot · 2024-10-24T21:49:46Z

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

…save`" Option only works in open source, not internal Differential Revision: [D64545647](https://our.internmc.facebook.com/intern/diff/D64545647) [ghstack-poisoned]

Pull Request resolved: #137735 Option only works in open source, not internal @imported-using-ghimport Differential Revision: [D64545647](https://our.internmc.facebook.com/intern/diff/D64545647/) ghstack-source-id: 249959423

facebook-github-bot · 2024-10-24T22:54:17Z

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

facebook-github-bot · 2024-10-25T19:31:03Z

@pytorchbot merge

(Initiating merge automatically since Phabricator Diff has merged)

pytorchmergebot · 2024-10-25T19:32:56Z

Merge failed

Reason: This PR has internal changes and must be landed via Phabricator! Please try reimporting/rexporting the PR!

Details for Dev Infra team

Raised by workflow job

wdvr · 2024-10-25T20:14:05Z

@pytorchmergebot merge -f "already landed internally"

pytorchmergebot · 2024-10-25T20:15:26Z

Merge started

Your change will be merged immediately since you used the force (-f) flag, bypassing any CI checks (ETA: 1-5 minutes). Please use -f as last resort and instead consider -i/--ignore-current to continue the merge ignoring current failures. This will allow currently pending tests to finish and report signal before the merge.

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

pytorchmergebot · 2024-10-25T20:15:36Z

Merge failed

Reason: This PR has internal changes and must be landed via Phabricator! Please try reimporting/rexporting the PR!

Details for Dev Infra team

Raised by workflow job

mikaylagawarecki · 2024-10-25T21:59:42Z

@pytorchmergebot merge -f "already landed internally"

pytorchmergebot · 2024-10-25T22:01:07Z

Merge started

Your change will be merged immediately since you used the force (-f) flag, bypassing any CI checks (ETA: 1-5 minutes). Please use -f as last resort and instead consider -i/--ignore-current to continue the merge ignoring current failures. This will allow currently pending tests to finish and report signal before the merge.

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

pytorchmergebot · 2024-10-25T22:01:19Z

Merge failed

Reason: This PR has internal changes and must be landed via Phabricator! Please try reimporting/rexporting the PR!

Details for Dev Infra team

Raised by workflow job

wdvr · 2024-10-25T23:47:21Z

as discussed with @mikaylagawarecki and @izaitsevfb - this has been reimported as a different PR to circumvent internal export check failures. I will close this one unmerged if that one is merged in.

@mikaylagawarecki

This is a cherry-pick from #137735 by @mikaylagawarecki , that cannot be merged due to a (wrongly) failing check for codev @diff-train-skip-merge Pull Request resolved: #138959 Approved by: https://github.com/mikaylagawarecki

Expose option to disable CRC-32 computation

7efbef4

[ghstack-poisoned]

pytorch-bot bot added the release notes: jit release notes category label Oct 10, 2024

mikaylagawarecki added a commit that referenced this pull request Oct 10, 2024

Expose option to disable CRC-32 computation

4e41f63

ghstack-source-id: 8f55cec Pull Request resolved: #137735

Update on "Expose option to disable CRC-32 computation"

21123ca

[ghstack-poisoned]

mikaylagawarecki changed the title ~~Expose option to disable CRC-32 computation~~ Expose option to disable CRC-32 computation during torch.save Oct 10, 2024

mikaylagawarecki added a commit that referenced this pull request Oct 10, 2024

Expose option to disable CRC-32 computation

935d8e3

ghstack-source-id: c98df38 Pull Request resolved: #137735

Update on "Expose option to disable CRC-32 computation during `torch.…

b13ee5f

…save`" [ghstack-poisoned]

mikaylagawarecki added a commit that referenced this pull request Oct 10, 2024

Expose option to disable CRC-32 computation

60edd72

ghstack-source-id: 6a551f4 Pull Request resolved: #137735

Update on "Expose option to disable CRC-32 computation during `torch.…

83169d9

…save`" [ghstack-poisoned]

mikaylagawarecki added release notes: python_frontend python frontend release notes category and removed release notes: jit release notes category labels Oct 11, 2024

mikaylagawarecki added a commit that referenced this pull request Oct 11, 2024

Expose option to disable CRC-32 computation

49909be

ghstack-source-id: b3bc417 Pull Request resolved: #137735

mikaylagawarecki added topic: new features topic category topic: improvements topic category and removed topic: new features topic category labels Oct 11, 2024

Update on "Expose option to disable CRC-32 computation during `torch.…

f8db3bd

…save`" [ghstack-poisoned]

mikaylagawarecki added a commit that referenced this pull request Oct 11, 2024

Expose option to disable CRC-32 computation

60a61fc

ghstack-source-id: 9184234 Pull Request resolved: #137735

Update on "Expose option to disable CRC-32 computation during `torch.…

021ac17

…save`" [ghstack-poisoned]

mikaylagawarecki added a commit that referenced this pull request Oct 11, 2024

Expose option to disable CRC-32 computation

a37cd86

ghstack-source-id: ef4eb95 Pull Request resolved: #137735

Update on "Expose option to disable CRC-32 computation during `torch.…

e801a47

…save`" [ghstack-poisoned]

mikaylagawarecki requested a review from albanD October 11, 2024 18:01

mikaylagawarecki added a commit that referenced this pull request Oct 11, 2024

Expose option to disable CRC-32 computation

71461fc

ghstack-source-id: 88e39a9 Pull Request resolved: #137735

mikaylagawarecki marked this pull request as ready for review October 11, 2024 18:01

albanD reviewed Oct 11, 2024

View reviewed changes

Update on "Expose option to disable CRC-32 computation during `torch.…

6d1f8f2

…save`" [ghstack-poisoned]

mikaylagawarecki added a commit that referenced this pull request Oct 11, 2024

Expose option to disable CRC-32 computation

04e5775

ghstack-source-id: 661896d Pull Request resolved: #137735

mikaylagawarecki requested a review from albanD October 11, 2024 19:33

Update on "Expose option to disable CRC-32 computation during `torch.…

f49bbe6

…save`" Option only works in open source, not internal [ghstack-poisoned]

mikaylagawarecki added a commit that referenced this pull request Oct 11, 2024

Expose option to disable CRC-32 computation

1ac8c37

ghstack-source-id: fe3cc74 Pull Request resolved: #137735

Update on "Expose option to disable CRC-32 computation during `torch.…

7365999

…save`" Option only works in open source, not internal Differential Revision: [D64545647](https://our.internmc.facebook.com/intern/diff/D64545647) [ghstack-poisoned]

mikaylagawarecki added a commit that referenced this pull request Oct 17, 2024

Expose option to disable CRC-32 computation

dd08b2a

ghstack-source-id: 40deb82 Pull Request resolved: #137735

Update on "Expose option to disable CRC-32 computation during `torch.…

a7d8b80

…save`" Option only works in open source, not internal Differential Revision: [D64545647](https://our.internmc.facebook.com/intern/diff/D64545647) [ghstack-poisoned]

facebook-github-bot added the fb-exported label Oct 24, 2024

Update on "Expose option to disable CRC-32 computation during `torch.…

8315e24

…save`" Option only works in open source, not internal Differential Revision: [D64545647](https://our.internmc.facebook.com/intern/diff/D64545647) [ghstack-poisoned]

pytorchmergebot added the merging label Oct 25, 2024

pytorchmergebot removed the merging label Oct 25, 2024

pytorchmergebot added the merging label Oct 25, 2024

pytorchmergebot removed the merging label Oct 25, 2024

pytorchmergebot added the merging label Oct 25, 2024

pytorchmergebot removed the merging label Oct 25, 2024

wdvr mentioned this pull request Oct 25, 2024

reimport pr137735 due to merging check issues #138959

Closed

wdvr closed this Oct 28, 2024

github-actions bot deleted the gh/mikaylagawarecki/272/head branch November 28, 2024 02:13

		return _compute_crc32


		def set_default_crc32_options(compute_crc32: bool):

Expose option to disable CRC-32 computation during torch.save #137735

Expose option to disable CRC-32 computation during torch.save #137735

Uh oh!

Conversation

mikaylagawarecki commented Oct 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pytorch-bot bot commented Oct 10, 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/137735

✅ No Failures

Uh oh!

albanD left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

albanD Oct 11, 2024

Choose a reason for hiding this comment

Uh oh!

mikaylagawarecki Oct 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

albanD Oct 11, 2024

Choose a reason for hiding this comment

Uh oh!

mikaylagawarecki Oct 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mikaylagawarecki commented Oct 17, 2024

Uh oh!

mikaylagawarecki commented Oct 17, 2024

Uh oh!

facebook-github-bot commented Oct 24, 2024

Uh oh!

facebook-github-bot commented Oct 24, 2024

Uh oh!

facebook-github-bot commented Oct 25, 2024

Uh oh!

pytorchmergebot commented Oct 25, 2024

Merge failed

Uh oh!

wdvr commented Oct 25, 2024

Uh oh!

pytorchmergebot commented Oct 25, 2024

Merge started

Uh oh!

pytorchmergebot commented Oct 25, 2024

Merge failed

Uh oh!

mikaylagawarecki commented Oct 25, 2024

Uh oh!

pytorchmergebot commented Oct 25, 2024

Merge started

Uh oh!

pytorchmergebot commented Oct 25, 2024

Merge failed

Uh oh!

wdvr commented Oct 25, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Expose option to disable CRC-32 computation during `torch.save` #137735

Expose option to disable CRC-32 computation during `torch.save` #137735

mikaylagawarecki commented Oct 10, 2024 •

edited

Loading

pytorch-bot bot commented Oct 10, 2024 •

edited

Loading

mikaylagawarecki Oct 11, 2024 •

edited

Loading

mikaylagawarecki Oct 11, 2024 •

edited

Loading