8000 Fix BytesWarning in torch.load() by aphedges · Pull Request #74813 · pytorch/pytorch · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Fix BytesWarning in torch.load() #74813

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 3 commits into from
Closed

Fix BytesWarning in torch.load() #74813

wants to merge 3 commits into from

Conversation

aphedges
Copy link
Contributor

Fixes #74812.

I have enabled the -bb option when tests are run to prevent regressions. I don't think it will make CI run more slowly, but I'm not entirely sure.

@facebook-github-bot
Copy link
Contributor

Hi @aphedges!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

@facebook-github-bot
Copy link
Contributor
facebook-github-bot commented Mar 27, 2022 •
edited
Loading

🔗 Helpful links

✅ No Failures (0 Pending)

As of commit 81f501a (more details on the Dr. CI page):

Expand to see more

💚 💚 Looks good so far! There are no failures yet. 💚 💚

This comment was automatically generated by Dr. CI (expand for details).

Please report bugs/suggestions to the (internal) Dr. CI Users group.

Click here to manually regenerate this comment.

@facebook-github-bot
Copy link
Contributor

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks!

@albanD albanD requested a review from malfet March 28, 2022 16:48
@albanD albanD added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Mar 28, 2022
@aphedges
Copy link
Contributor Author

Pinging @malfet (according to https://github.com/pytorch/pytorch/wiki/Developer-FAQ#my-pr-hasnt-been-reviewed-in-a-while-what-should-i-do).

@aphedges
Copy link
Contributor Author

Pinging @malfet again. I would really appreciate a review.

@aphedges
Copy link
Contributor Author

@kit1980, thank you very much!

@kit1980
Copy link
Contributor
kit1980 commented Jun 16, 2022

@pytorchbot rebase

@pytorchmergebot
Copy link
Collaborator

@pytorchbot successfully started a rebase job. Check the current status here

@pytorchmergebot
Copy link
Collaborator

Successfully rebased 74812-fix-bytes-warning onto refs/remotes/origin/master, please pull locally before adding more changes (for example, via git checkout 74812-fix-bytes-warning && git pull --rebase)

@pytorchmergebot pytorchmergebot force-pushed the 74812-fix-bytes-warning branch from f1186a7 to 5750815 Compare June 16, 2022 04:26
@aphedges aphedges requested a review from a team as a code owner June 16, 2022 04:26
@kit1980
Copy link
Contributor
kit1980 commented Jun 16, 2022

@aphedges I see now at least one test is failing, can you also update this?

  File "/opt/conda/lib/python3.7/site-packages/torch/testing/_internal/common_device_type.py", line 377, in instantiated_test
    result = test(self, **param_kwargs)
  File "test_torch.py", line 751, in test_cpp_warnings_have_python_context
    self.assertTrue(re.search(s, str(escaped_warning_message), re.IGNORECASE) is not None)
BytesWarning: str() on a bytes instance

@aphedges
Copy link
Contributor Author

@kit1980, I'm not surprised that a failure happened, given that it's been so long. I'll push a fix within the next couple of days.

@aphedges
Copy link
Contributor Author

This failure was caused by 2d4291f, but it's unclear why the change was made in the first place because it was a failure on the commit author's machine, not in CI. I asked for clarification at #77531 (comment), but I have no clue if I'll get a response.

In the meantime, I'll push my fix and see if tests now pass.

@aphedges aphedges force-pushed the 74812-fix-bytes-warning branch from 5750815 to 81f501a Compare June 17, 2022 19:50
@kit1980
Copy link
Contributor
kit1980 commented Jun 17, 2022

Let's land this.

@kit1980
Copy link
Contributor
kit1980 commented Jun 17, 2022

@pytorchbot merge

@pytorchmergebot
Copy link
Collaborator

@pytorchbot successfully started a merge job. Check the current status here

@github-actions
Copy link
Contributor

Hey @aphedges.
You've committed this PR, but it does not have both a 'release notes: ...' and 'topics: ...' label. Please add one of each to the PR. The 'release notes: ...' label should represent the part of PyTorch that this PR changes (fx, autograd, distributed, etc) and the 'topics: ...' label should represent the kind of PR it is (not user facing, new feature, bug fix, perf improvement, etc). The list of valid labels can be found here for the 'release notes: ...' and here for the 'topics: ...'.
For changes that are 'topic: not user facing' there is no need for a release notes label.

@aphedges
Copy link
Contributor Author

@kit1980, thank you very much!

@janeyx99
Copy link
Contributor

@aphedges Sorry, but your change broke our slow tests that run on trunk only. :( When you reland this, please ensure the slow tests are run and pass by adding the ciflow/trunk label to your pull request.

@pytorchbot revert -m "Broke slow tests in cuda 10.2 https://github.com/pytorch/pytorch/runs/6944238177?check_suite_focus=true" -c nosignal

@pytorchmergebot
Copy link
Collaborator

@pytorchbot successfully started a revert job. Check the current status here

@pytorchmergebot
Copy link
Collaborator

Reverting PR 74813 failed due to Comment @aphedges Sorry, but your change broke our slow tests that run on trunk only. :( When you reland this, please ensure the slow tests are run and pass by adding the ciflow/trunk label to your pull request.
@pytorchbot revert -m "Broke slow tests in cuda 10.2 https://github.com/pytorch/pytorch/runs/6944238177?check_suite_focus=true" -c nosignal does not seem to be a valid revert command
Raised by https://github.com/pytorch/pytorch/actions/runs/2519111854

@janeyx99
Copy link
Contributor

@pytorchbot revert -m "Broke slow tests in cuda 10.2 https://github.com/pytorch/pytorch/runs/6944238177?check_suite_focus=true" -c nosignal

@pytorchmergebot
Copy link
Collaborator

@pytorchbot successfully started a revert job. Check the current status here

pytorchmergebot added a commit that referenced this pull request Jun 18, 2022
@pytorchmergebot
Revert "Fix BytesWarning in torch.load() (#74813)"
e10cbe3 
This reverts commit 6c2e811.

Reverted #74813 on behalf of https://github.com/janeyx99 due to Broke slow tests in cuda 10.2 https://github.com/pytorch/pytorch/runs/6944238177?check_suite_focus=true
This was referenced Jun 18, 2022
Closed
Closed
@zengk95 zengk95 mentioned this pull request Jun 22, 2022
Closed
miladm pushed a commit to miladm/pytorch that referenced this pull request Jun 27, 2022
@aphedges @miladm
Fix BytesWarning in torch.load() (pytorch#74813)
6b7bfcc 
Fixes pytorch#74812.

I have enabled the `-bb` option when tests are run to prevent regressions. I don't think it will make CI run more slowly, but I'm not entirely sure.

Pull Request resolved: pytorch#74813
Approved by: https://github.com/kit1980
miladm pushed a commit to miladm/pytorch that referenced this pull request Jun 27, 2022
@pytorchmergebot @miladm
Revert "Fix BytesWarning in torch.load() (pytorch#74813)"
c6b2a0f 
This reverts commit 6c2e811.

Reverted pytorch#74813 on behalf of https://github.com/janeyx99 due to Broke slow tests in cuda 10.2 https://github.com/pytorch/pytorch/runs/6944238177?check_suite_focus=true
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kit1980 kit1980 kit1980 approved these changes

@malfet malfet Awaiting requested review from malfet

Assignees
No one assigned
Labels
cla signed Merged open source Reverted triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

BytesWarning causes exception in torch.load() when running Python with -bb flag
7 participants
@aphedges @facebook-github-bot @kit1980 @pytorchmergebot @janeyx99 @albanD @pytorchbot
0