Skip to content

functionalization: fix bug with multiple views of same base#77129

Closed
bdhirsh wants to merge 16 commits intogh/bdhirsh/227/basefrom
gh/bdhirsh/227/head
Closed

functionalization: fix bug with multiple views of same base#77129
bdhirsh wants to merge 16 commits intogh/bdhirsh/227/basefrom
gh/bdhirsh/227/head

Conversation

@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented May 10, 2022

🔗 Helpful links

✅ No Failures (0 Pending)

As of commit 9c04725 (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.

@bdhirsh bdhirsh requested a review from ezyang May 11, 2022 14:23
regenerate_from_base();
}
apply_updates();
regenerate_from_base();
Copy link
Contributor

Choose a reason for hiding this comment

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

Now you get to explain, in the code here, why skipping regeneration when there are no updates is wrong ;)

Copy link
Contributor

@ezyang ezyang left a comment

Choose a reason for hiding this comment

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

I figured it out looking at the test, haha. Nice catch.

bdhirsh added 3 commits May 11, 2022 07:31
welp, I realized my "perf improvement" from #75819 was wrong. This originally came up because `functionalize()` was sometimes emit an unnecessary `view_copy`, but there's a more correct fix that I can make inside of functorch, which I have here: pytorch/functorch#795






[ghstack-poisoned]
welp, I realized my "perf improvement" from #75819 was wrong. This originally came up because `functionalize()` was sometimes emit an unnecessary `view_copy`, but there's a more correct fix that I can make inside of functorch, which I have here: pytorch/functorch#795






[ghstack-poisoned]
welp, I realized my "perf improvement" from #75819 was wrong. This originally came up because `functionalize()` was sometimes emit an unnecessary `view_copy`, but there's a more correct fix that I can make inside of functorch, which I have here: pytorch/functorch#795






[ghstack-poisoned]
bdhirsh added 4 commits May 17, 2022 19:58
welp, I realized my "perf improvement" from #75819 was wrong. This originally came up because `functionalize()` was sometimes emit an unnecessary `view_copy`, but there's a more correct fix that I can make inside of functorch, which I have here: pytorch/functorch#795






[ghstack-poisoned]
welp, I realized my "perf improvement" from #75819 was wrong. This originally came up because `functionalize()` was sometimes emit an unnecessary `view_copy`, but there's a more correct fix that I can make inside of functorch, which I have here: pytorch/functorch#795






[ghstack-poisoned]
welp, I realized my "perf improvement" from #75819 was wrong. This originally came up because `functionalize()` was sometimes emit an unnecessary `view_copy`, but there's a more correct fix that I can make inside of functorch, which I have here: pytorch/functorch#795






[ghstack-poisoned]
welp, I realized my "perf improvement" from #75819 was wrong. This originally came up because `functionalize()` was sometimes emit an unnecessary `view_copy`, but there's a more correct fix that I can make inside of functorch, which I have here: pytorch/functorch#795






[ghstack-poisoned]
bdhirsh added 7 commits May 19, 2022 07:37
welp, I realized my "perf improvement" from #75819 was wrong. This originally came up because `functionalize()` was sometimes emit an unnecessary `view_copy`, but there's a more correct fix that I can make inside of functorch, which I have here: pytorch/functorch#795






[ghstack-poisoned]
welp, I realized my "perf improvement" from #75819 was wrong. This originally came up because `functionalize()` was sometimes emit an unnecessary `view_copy`, but there's a more correct fix that I can make inside of functorch, which I have here: pytorch/functorch#795






[ghstack-poisoned]
welp, I realized my "perf improvement" from #75819 was wrong. This originally came up because `functionalize()` was sometimes emit an unnecessary `view_copy`, but there's a more correct fix that I can make inside of functorch, which I have here: pytorch/functorch#795






[ghstack-poisoned]
welp, I realized my "perf improvement" from #75819 was wrong. This originally came up because `functionalize()` was sometimes emit an unnecessary `view_copy`, but there's a more correct fix that I can make inside of functorch, which I have here: pytorch/functorch#795






[ghstack-poisoned]
welp, I realized my "perf improvement" from #75819 was wrong. This originally came up because `functionalize()` was sometimes emit an unnecessary `view_copy`, but there's a more correct fix that I can make inside of functorch, which I have here: pytorch/functorch#795






[ghstack-poisoned]
welp, I realized my "perf improvement" from #75819 was wrong. This originally came up because `functionalize()` was sometimes emit an unnecessary `view_copy`, but there's a more correct fix that I can make inside of functorch, which I have here: pytorch/functorch#795






[ghstack-poisoned]
welp, I realized my "perf improvement" from #75819 was wrong. This originally came up because `functionalize()` was sometimes emit an unnecessary `view_copy`, but there's a more correct fix that I can make inside of functorch, which I have here: pytorch/functorch#795






[ghstack-poisoned]
facebook-github-bot pushed a commit that referenced this pull request May 26, 2022
Summary:
Pull Request resolved: #77129

Approved by: https://github.com/ezyang

Test Plan: contbuild & OSS CI, see https://hud.pytorch.org/commit/pytorch/pytorch/2eea5eff622df108f2f77dda61ddec3a62b51e51

Reviewed By: mehtanirav

Differential Revision: D36668391

Pulled By: bdhirsh

fbshipit-source-id: 4ab5fc8db24b83b7b1c8bfe17f82d687e64ad7bc
@facebook-github-bot facebook-github-bot deleted the gh/bdhirsh/227/head branch May 28, 2022 14:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants