Skip to content

[Placeholder] Fix a differentiation bug related to Placeholders. #1627

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.

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

Merged
merged 1 commit into from
Sep 12, 2018

Conversation

nadavrot
Copy link
Contributor

Description:

Fix a differentiation bug related to Placeholders. The line assumed that
the destination is a variable.

Testing:

This bug was exposed in PR #1622. I did not add a specific test for this because as soon as we migrate more tests to using Placeholders this code will be tested.

Documentation: None

Fix a differentiation bug related to Placeholders. The line assumed that
the destination is a variable.
Copy link
Contributor

@artemrakhov-glow artemrakhov-glow left a comment

Choose a reason for hiding this comment

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

Should we get rid of getVariable()?

@nadavrot
Copy link
Contributor Author

@artemrakhov Yes, but we can't get rid of getVariable until we get rid of Variables. :)
I am taking small steps right now because soon we'll need to rewrite ALL of our unit tests to using Placeholder, instead of Variables, and this will be a huge change, and I am trying to prepare for this big change with small preparation changes. In this PR: #1622 I tested the porting of MNIST. Things worked but I ran into a few things that I am now fixing. After migrating everything from Variable to Placeholder we'll be able to delete Storage and just keep Placeholder and Constant.

@nadavrot nadavrot merged commit ae80909 into pytorch:master Sep 12, 2018
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.

4 participants