Skip to content
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

[jit] Warn when tracing legacy constructors #16770

Closed
wants to merge 2 commits into from
Closed

[jit] Warn when tracing legacy constructors #16770

wants to merge 2 commits into from

Conversation

driazati
Copy link
Contributor

@driazati driazati commented Feb 5, 2019

No description provided.

@facebook-github-bot facebook-github-bot added the oncall: jit Add this issue/PR to JIT oncall triage queue label Feb 5, 2019
@suo
Copy link
Member

suo commented Feb 5, 2019

We should change the LSTM implementation before landing this.

facebook-github-bot
facebook-github-bot reviewed Feb 5, 2019
View reviewed changes
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

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

@fritzo
Copy link
Collaborator

fritzo commented Apr 30, 2019

@driazati What is the recommended replacement? Should we replace
x.new_foo(shape) -> torch.foo(shape, dtype=x.dtype, device=x.device)?
See also #18434

@fritzo
Copy link
Collaborator

fritzo commented Apr 30, 2019

@neerajprad We should also avoid legacy constructors in torch.distributions.

@fritzo
Copy link
Collaborator

fritzo commented Apr 30, 2019 •
edited

@soumith Is there a chance we can revert this before the 1.1 release? There is still heavy usage of the .new_*() constructors in torch.distributions, and I'd like to avoid burdening jit users with deprecation warnings due to inconsistency across torch modules. Alternatively, @neerajprad offered to update torch.distributions before the 1.1 release.

cc @colesbury

@soumith
Copy link
Member

soumith commented Apr 30, 2019

the 1.1 release is just about going live now. i think it's a bit too late, but let me check

@fritzo fritzo mentioned this pull request Apr 30, 2019
Merged
4 tasks
@neerajprad neerajprad mentioned this pull request Apr 30, 2019
Closed
@ssnl
Copy link
Collaborator

ssnl commented May 1, 2019

Hmm we introduced these in 0.4 and heavily advertised them and now they are legacy?!!

@ssnl ssnl mentioned this pull request May 1, 2019
Closed
@driazati driazati mentioned this pull request May 1, 2019
Closed
facebook-github-bot pushed a commit that referenced this pull request May 1, 2019
@facebook-github-bot
Remove warnings on new_* constructors (#20026)
18cb098 
Summary:
Stack from [ghstack](https://github.com/ezyang/ghstack):
* **#20026 Remove warnings on new_* constructors**

Revert of #16770, fixes #19995
Pull Request resolved: #20026

Pulled By: driazati

Differential Revision: D15171691

fbshipit-source-id: 057c3b4a9fd6086ca240007e5404a286080f04b6
facebook-github-bot pushed a commit that referenced this pull request May 3, 2019
@neerajprad @facebook-github-bot
Remove deprecated tensor constructors in torch.distributions (#19979)
fb40e58 
Summary:
This removes the deprecated `tensor.new_*` constructors (see #16770) from `torch.distributions` module.
Pull Request resolved: #19979

Differential Revision: D15195618

Pulled By: soumith

fbshipit-source-id: 46b519bfd32017265e90bd5c53f12cfe4a138021
zhangguanheng66 pushed a commit to zhangguanheng66/pytorch that referenced this pull request May 6, 2019
Remove warnings on new_* constructors (pytorch#20026)
2c14bdf 
Summary:
Stack from [ghstack](https://github.com/ezyang/ghstack):
* **pytorch#20026 Remove warnings on new_* constructors**

Revert of pytorch#16770, fixes pytorch#19995
Pull Request resolved: pytorch#20026

Pulled By: driazati

Differential Revision: D15171691

fbshipit-source-id: 057c3b4a9fd6086ca240007e5404a286080f04b6
@ezyang ezyang added the merged label Jun 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@facebook-github-bot facebook-github-bot facebook-github-bot left review comments

@suo suo suo approved these changes

Assignees
No one assigned
Labels
oncall: jit Add this issue/PR to JIT oncall triage queue
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@driazati @suo @fritzo @soumith @ssnl @facebook-github-bot @ezyang