Skip to content

Reduce number of variables in codegen#38369

Closed
smessmer wants to merge 2 commits intogh/smessmer/221/basefrom
gh/smessmer/221/head
Closed

Reduce number of variables in codegen#38369
smessmer wants to merge 2 commits intogh/smessmer/221/basefrom
gh/smessmer/221/head

Conversation

@smessmer
Copy link
Copy Markdown
Contributor

@smessmer smessmer commented May 13, 2020 •
edited
Loading

Stack from ghstack:

Seems we have a lot of variables in codegen that carry duplicate information.
This PR is removing them. It unifies all use sites to use the same instance

Differential Revision: D21537983

@smessmer
Reduce number of variables in codegen (1/2)
82c9161 
Seems we have a lot of variables in codegen that carry duplicate information.
This PR is step 1/2 of removing them. It unifies all use sites to use the same instance
and adds assertions to proof that both instances actually carry the same data.
If this lands successfully, then a future PR will remove the other instances and the assertions.

Differential Revision: [D21537983](https://our.internmc.facebook.com/intern/diff/D21537983/)

[ghstack-poisoned]
smessmer added a commit that referenced this pull request May 13, 2020
@smessmer
Reduce number of variables in codegen (1/2)
6824b4a 
Seems we have a lot of variables in codegen that carry duplicate information.
This PR is step 1/2 of removing them. It unifies all use sites to use the same instance
and adds assertions to proof that both instances actually carry the same data.
If this lands successfully, then a future PR will remove the other instances and the assertions.

Differential Revision: [D21537983](https://our.internmc.facebook.com/intern/diff/D21537983/)

ghstack-source-id: 103980358
Pull Request resolved: #38369
@dr-ci
Copy link
Copy Markdown

dr-ci Bot commented May 13, 2020 •
edited
Loading

💊 CI failures summary and remediations

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

❄️ 1 failure tentatively classified as flaky

but reruns have not yet been triggered to confirm:

See CircleCI build pytorch_windows_vs2019_py36_cuda10.1_on_cpu_test1 (1/1)

Step: "Test" (full log | diagnosis details | 🔁 rerun) ❄️

pip._vendor.urllib3.exceptions.ProtocolError: ("Connection broken: ConnectionResetError(10054, 'An existing connection was forcibly closed by the remote host', None, 10054, None)", ConnectionResetError(10054, 'An existing connection was forcibly closed by the remote host', None, 10054, None)) 
  File "C:\Jenkins\Miniconda3\lib\site-packages\pip\_internal\network\utils.py", line 39, in response_chunks 
    decode_content=False, 
  File "C:\Jenkins\Miniconda3\lib\site-packages\pip\_vendor\urllib3\response.py", line 564, in stream 
    data = self.read(amt=amt, decode_content=decode_content) 
  File "C:\Jenkins\Miniconda3\lib\site-packages\pip\_vendor\urllib3\response.py", line 529, in read 
    raise IncompleteRead(self._fp_bytes_read, self.length_remaining) 
  File "C:\Jenkins\Miniconda3\lib\contextlib.py", line 99, in __exit__ 
    self.gen.throw(type, value, traceback) 
  File "C:\Jenkins\Miniconda3\lib\site-packages\pip\_vendor\urllib3\response.py", line 443, in _error_catcher 
    raise ProtocolError("Connection broken: %r" % e, e) 
pip._vendor.urllib3.exceptions.ProtocolError: ("Connection broken: ConnectionResetError(10054, 'An existing connection was forcibly closed by the remote host', None, 10054, None)", ConnectionResetError(10054, 'An existing connection was forcibly closed by the remote host', None, 10054, None)) 
 
(base) circleci@PACKER-5E9D8C4B C:\Users\circleci\project>pushd .  
 
(base) circleci@PACKER-5E9D8C4B C:\Users\circleci\project>if "" == "" (call "C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Auxiliary\Build\vcvarsall.bat" x64 )  else (call "C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Auxiliary\Build\vcvarsall.bat" x64 -vcvars_ver= )  
********************************************************************** 
** Visual Studio 2019 Developer Command Prompt v16.5.4 
** Copyright (c) 2019 Microsoft Corporation 
********************************************************************** 
[vcvarsall.bat] Environment initialized for: 'x64'

ci.pytorch.org: 1 failed

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 on the GitHub issue tracker.

See how this bot performed.

This comment has been revised 5 times.

@smessmer
Update on "Reduce number of variables in codegen (1/2)"
f1a1108 
Seems we have a lot of variables in codegen that carry duplicate information.
This PR is step 1/2 of removing them. It unifies all use sites to use the same instance
and adds assertions to proof that both instances actually carry the same data.
If this lands successfully, then a future PR will remove the other instances and the assertions.

Differential Revision: [D21537983](https://our.internmc.facebook.com/intern/diff/D21537983/)

[ghstack-poisoned]
smessmer added a commit that referenced this pull request May 14, 2020
@smessmer
Reduce number of variables in codegen
c2523ef 
Pull Request resolved: #38369
Seems we have a lot of variables in codegen that carry duplicate information.
This PR is removing them. It unifies all use sites to use the same instance

ghstack-source-id: 104067031

Differential Revision: [D21537983](https://our.internmc.facebook.com/intern/diff/D21537983/)
@smessmer smessmer changed the title Reduce number of variables in codegen (1/2) Reduce number of variables in codegen May 14, 2020
@smessmer smessmer requested review from bhosmer and ezyang May 14, 2020 00:36
ezyang
ezyang approved these changes May 14, 2020
View reviewed changes
Copy link
Copy Markdown
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.

Thanks. Could you please diff the generated files before and after and verify there are no changes?

@smessmer
Copy link
Copy Markdown
Contributor Author

smessmer commented May 15, 2020

Thanks. Could you please diff the generated files before and after and verify there are no changes?

confirmed

@facebook-github-bot
Copy link
Copy Markdown
Contributor

facebook-github-bot commented May 16, 2020

This pull request has been merged in feb2457.

@facebook-github-bot facebook-github-bot deleted the gh/smessmer/221/head branch May 19, 2020 14:16
laurentdupin pushed a commit to laurentdupin/pytorch that referenced this pull request Apr 24, 2026
@smessmer @facebook-github-bot
Reduce number of variables in codegen (pytorch#38369)
427a5e1 
Summary:
Pull Request resolved: pytorch#38369
Seems we have a lot of variables in codegen that carry duplicate information.
This PR is removing them. It unifies all use sites to use the same instance

ghstack-source-id: 104067031

Test Plan: waitforsandcastle

Differential Revision: D21537983

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

Reviewers

@ezyang ezyang ezyang approved these changes

@bhosmer bhosmer Awaiting requested review from bhosmer

Assignees

No one assigned

Labels

Merged

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@smessmer @facebook-github-bot @ezyang @mruberry