Skip to content
This repository was archived by the owner on Aug 21, 2025. It is now read-only.

make sync logic more accurate for functionalization#795

Merged
bdhirsh merged 1 commit intomainfrom
make_sync_logic_more_accurate
Jul 13, 2022
Merged

make sync logic more accurate for functionalization#795
bdhirsh merged 1 commit intomainfrom
make_sync_logic_more_accurate

Conversation

@bdhirsh
Copy link
Contributor

@bdhirsh bdhirsh commented May 11, 2022

Richard left a comment here a while ago that I forgot about: #235 (comment)

I had a crappy fix for ensuring that functionalize() doesn't emit extra unnecessary view_copy ops that was actually wrong in some cases, but this should really fix it

@bdhirsh bdhirsh requested a review from zou3519 May 11, 2022 14:22
bdhirsh added a commit to pytorch/pytorch that referenced this pull request May 11, 2022
…iews of same base"

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 a commit to pytorch/pytorch that referenced this pull request May 11, 2022
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 a commit to pytorch/pytorch that referenced this pull request May 13, 2022
…iews of same base"

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 a commit to pytorch/pytorch that referenced this pull request May 13, 2022
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]
Copy link
Contributor

@zou3519 zou3519 left a comment

Choose a reason for hiding this comment

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

Sure. Is it possible to add a test?

bdhirsh added a commit to pytorch/pytorch that referenced this pull request May 18, 2022
…iews of same base"

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 a commit to pytorch/pytorch that referenced this pull request May 18, 2022
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 a commit to pytorch/pytorch that referenced this pull request May 18, 2022
…iews of same base"

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 a commit to pytorch/pytorch that referenced this pull request May 18, 2022
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 a commit to pytorch/pytorch that referenced this pull request May 18, 2022
…iews of same base"

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 a commit to pytorch/pytorch that referenced this pull request May 18, 2022
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 a commit to pytorch/pytorch that referenced this pull request May 18, 2022
…iews of same base"

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 a commit to pytorch/pytorch that referenced this pull request May 18, 2022
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 a commit to pytorch/pytorch that referenced this pull request May 19, 2022
…iews of same base"

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 a commit to pytorch/pytorch that referenced this pull request May 19, 2022
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 a commit to pytorch/pytorch that referenced this pull request May 19, 2022
…iews of same base"

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 a commit to pytorch/pytorch that referenced this pull request May 19, 2022
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 a commit to pytorch/pytorch that referenced this pull request May 19, 2022
…iews of same base"

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 a commit to pytorch/pytorch that referenced this pull request May 19, 2022
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 a commit to pytorch/pytorch that referenced this pull request May 20, 2022
…iews of same base"

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 a commit to pytorch/pytorch that referenced this pull request May 20, 2022
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 a commit to pytorch/pytorch that referenced this pull request May 20, 2022
…iews of same base"

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 a commit to pytorch/pytorch that referenced this pull request May 20, 2022
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 a commit to pytorch/pytorch that referenced this pull request May 23, 2022
…iews of same base"

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 a commit to pytorch/pytorch that referenced this pull request May 23, 2022
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 a commit to pytorch/pytorch that referenced this pull request May 24, 2022
…iews of same base"

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 a commit to pytorch/pytorch that referenced this pull request May 24, 2022
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 a commit to pytorch/pytorch that referenced this pull request May 24, 2022
…iews of same base"

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 a commit to pytorch/pytorch that referenced this pull request May 24, 2022
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]
@zou3519
Copy link
Contributor

zou3519 commented Jul 13, 2022

@bdhirsh can we merge this? I'm not sure if anything has changed

@bdhirsh
Copy link
Contributor Author

bdhirsh commented Jul 13, 2022

yes, we should merge this. I'll rebase and re-run tests for sanity

@bdhirsh bdhirsh force-pushed the make_sync_logic_more_accurate branch from 7c49c96 to 077d5d5 Compare July 13, 2022 17:41
@bdhirsh
Copy link
Contributor Author

bdhirsh commented Jul 13, 2022

checks pass so I"ll merge :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants