Skip to content

Conversation

@zdevito
Copy link
Contributor

@zdevito zdevito commented Jun 3, 2019

Stack from ghstack:

Summary: this flag is used in tests to enable first-class module
execution so that we can test things like invoking methods on
first-class modules and setting their properties before we implement
all the parts necessary to remove the old execution pattern.

Differential Revision: D15600068

Summary: this flag is used in tests to enable first-class module
execution so that we can test things like invoking methods on
first-class modules and setting their properties before we implement
all the parts necessary to remove the old execution pattern.
Add flag to temporarily enable first class modules

Summary: this flag is used in tests to enable first-class module
execution so that we can test things like invoking methods on
first-class modules and setting their properties before we implement
all the parts necessary to remove the old execution pattern.

gh-metadata: pytorch pytorch 21272 gh/zdevito/46/head
Add flag to temporarily enable first class modules

Summary: this flag is used in tests to enable first-class module
execution so that we can test things like invoking methods on
first-class modules and setting their properties before we implement
all the parts necessary to remove the old execution pattern.

gh-metadata: pytorch pytorch 21272 gh/zdevito/46/head
@ezyang ezyang added facebook and removed facebook labels Jun 5, 2019
Copy link
Contributor

@Krovatkin Krovatkin left a comment

Choose a reason for hiding this comment

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

:shipit:

foo = Foo()
input = torch.rand(3, 4)
foo.forward(input)
self.assertEqual(input, foo.foo)
Copy link
Contributor

Choose a reason for hiding this comment

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

nitpick: foo.bar might be a little bit more readable


IValue Method::operator()(std::vector<IValue> stack, const Kwargs& kwargs) {
getSchema().checkAndNormalizeInputs(stack, kwargs);
if (experimental_run_as_first_class) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nitpick: this logic (pushing module and parameters onto a stack) seems to be duplicated across run and (). Maybe we should consider refactoring it into a helper, otherwise we might forget to update one of the places eventually?


struct TORCH_API Method {
Method(Module* owner, Function* function);
Method(Module* owner, const std::shared_ptr<Function>& function);
Copy link
Contributor

Choose a reason for hiding this comment

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

so someone else now owns a function aside from Method?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

for now yes, Michael is working on our final state for Function ownership which will make this stuff more clear.

Add flag to temporarily enable first class modules

Summary: this flag is used in tests to enable first-class module
execution so that we can test things like invoking methods on
first-class modules and setting their properties before we implement
all the parts necessary to remove the old execution pattern.

gh-metadata: pytorch pytorch 21272 gh/zdevito/46/head
Zachary DeVito and others added 2 commits June 7, 2019 10:28
Add flag to temporarily enable first class modules

Summary: this flag is used in tests to enable first-class module
execution so that we can test things like invoking methods on
first-class modules and setting their properties before we implement
all the parts necessary to remove the old execution pattern.

gh-metadata: pytorch pytorch 21272 gh/zdevito/46/head
Add flag to temporarily enable first class modules

Summary: this flag is used in tests to enable first-class module
execution so that we can test things like invoking methods on
first-class modules and setting their properties before we implement
all the parts necessary to remove the old execution pattern.

gh-metadata: pytorch pytorch 21272 gh/zdevito/46/head
@zou3519 zou3519 deleted the gh/zdevito/46/head branch June 8, 2019 03:59
@facebook-github-bot
Copy link
Contributor

@zdevito merged this pull request in 7e08bc4.

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

Labels

Merged oncall: jit Add this issue/PR to JIT oncall triage queue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants