Skip to content

Conversation

@eellison
Copy link
Contributor

@eellison eellison commented Sep 5, 2019

Add support for nn.ModuleDict in script. This is needed to support torchvision.

@eellison eellison requested review from driazati and suo September 5, 2019 16:57
@eellison eellison requested a review from apaszke as a code owner September 5, 2019 16:57
@pytorchbot pytorchbot added oncall: jit Add this issue/PR to JIT oncall triage queue module: nn Related to torch.nn module: pybind Related to our Python bindings / interactions with other Python libraries labels Sep 5, 2019
Copy link
Contributor

@driazati driazati left a comment

Choose a reason for hiding this comment

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

Mostly good, a couple small questions

x = mod(x)

for mod in self.moduledict.values():
x = mod(x)
Copy link
Contributor

Choose a reason for hiding this comment

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

This only checks the value is right for the last module in the dict, the results should go into a list or something so they can all be checked

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, the three modules that i have aren't commutative so it's checking that the order is correct.

test/test_jit.py Outdated

return x, names

m = M()
Copy link
Contributor

Choose a reason for hiding this comment

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

checkModule does this same thing

def checkModule(self, nn_module, args):

@eellison eellison requested a review from driazati September 10, 2019 22:34
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.

@eellison is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@eellison merged this pull request in 8f7020b.

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

Labels

Merged module: nn Related to torch.nn module: pybind Related to our Python bindings / interactions with other Python libraries oncall: jit Add this issue/PR to JIT oncall triage queue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants