Skip to content

Conversation

@soulitzer
Copy link
Contributor

@soulitzer soulitzer commented Apr 14, 2021

Fixes #50617

BC-breaking Notes

In-place modification of views will now raise an error if that view was created by a custom function or a function that returns multiple views, or if the view was created in no-grad mode.

Modifying in-place a view created in the situations above are error-prone and have been deprecated since v1.5.0. Doing these in-place modifications are now forbidden.

For more information on how to work around this, see the related sections the release notes linked below:

  • v1.5.0 (view created in custom autograd function, view created in no-grad block)
  • v1.7.0 (section on split and chunk, i.e., functions that return multiple views).

-- end release notes --
This PR also updates the relevant tests to expect errors instead of warnings

@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Apr 14, 2021

💊 CI failures summary and remediations

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


  • 1/3 failures possibly* introduced in this PR
    • 1/1 non-scanned failure(s)
  • 2/3 broken upstream at merge base 6c327ef on Apr 15 from 11:09am to 4:28pm

🚧 2 fixed upstream failures:

These were probably caused by upstream breakages that were already fixed.

Please rebase on the viable/strict branch (expand for instructions)

If your commit is older than viable/strict, run these commands:

git fetch https://github.com/pytorch/pytorch viable/strict
git rebase FETCH_HEAD

This comment was automatically generated by Dr. CI (expand for details).Follow this link to opt-out of these comments for your Pull Requests.

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

@soulitzer soulitzer force-pushed the inplace-view-error branch 4 times, most recently from 7c476bc to 435aa3b Compare April 15, 2021 03:14
@codecov
Copy link

codecov bot commented Apr 15, 2021

Codecov Report

Merging #56093 (435aa3b) into master (0b8bd22) will increase coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head 435aa3b differs from pull request most recent head f04bc30. Consider uploading reports for the commit f04bc30 to get more accurate results

@@           Coverage Diff           @@
##           master   #56093   +/-   ##
=======================================
  Coverage   77.12%   77.12%           
=======================================
  Files        1912     1912           
  Lines      189559   189557    -2     
=======================================
- Hits       146195   146194    -1     
+ Misses      43364    43363    -1     

Copy link
Collaborator

@albanD albanD left a comment

Choose a reason for hiding this comment

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

Looks mostly good! Thanks

@soulitzer soulitzer force-pushed the inplace-view-error branch 2 times, most recently from 2fa5fb8 to 06415cb Compare April 15, 2021 18:19
@soulitzer soulitzer requested a review from albanD April 15, 2021 19:20
@soulitzer soulitzer force-pushed the inplace-view-error branch from 06415cb to f04bc30 Compare April 15, 2021 19:43
Copy link
Collaborator

@albanD albanD left a comment

Choose a reason for hiding this comment

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

lgtm

@facebook-github-bot
Copy link
Contributor

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

@facebook-github-bot
Copy link
Contributor

@soulitzer merged this pull request in dd8bfe2.

@soulitzer soulitzer added the module: bc-breaking Related to a BC-breaking change label Apr 20, 2021
krshrimali pushed a commit to krshrimali/pytorch that referenced this pull request May 19, 2021
Summary:
Fixes pytorch#50617

Also updates the relevant tests to expect errors instead of warnings

Pull Request resolved: pytorch#56093

Reviewed By: agolynski

Differential Revision: D27806795

Pulled By: soulitzer

fbshipit-source-id: 93c5c28edb1f97fa4457332c2ef4711f050ac81f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla signed Merged module: bc-breaking Related to a BC-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Finish deprecation cycle for inplace view error checks

3 participants