Skip to content

Conversation

@zdevito
Copy link
Contributor

@zdevito zdevito commented Jun 3, 2019

Stack from ghstack:

Summary: This reorganizes GraphExecutor into a stage that looks up
the execution plan getPlanFor, and a stage that runs the plan. This
separation is necessary for first-class functions since the interpreter
will call getPlanFor to look up the Code for the function it will be
calling rather than recursively calling run on the graph executor.

Test Plan: test_jit.py

Differential Revision: D15600067

Summary: This reorganizes GraphExecutor into a stage that looks up
the execution plan getPlanFor, and a stage that runs the plan. This
separation is necessary for first-class functions since the interpreter
will call getPlanFor to look up the Code for the function it will be
calling rather than recursively calling run on the graph executor.

Test Plan: test_jit.py
Expose ExecutionPlan in prep for function calls
Summary: This reorganizes GraphExecutor into a stage that looks up
the execution plan getPlanFor, and a stage that runs the plan. This
separation is necessary for first-class functions since the interpreter
will call getPlanFor to look up the Code for the function it will be
calling rather than recursively calling run on the graph executor.

Test Plan: test_jit.py

gh-metadata: pytorch pytorch 21273 gh/zdevito/47/head
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:


// entry point where execution begins
virtual void run(Stack& stack) = 0;
void run(Stack& stack);
Copy link
Contributor

Choose a reason for hiding this comment

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

i think we might need virtual GraphExecutorImpl::run eventually, but I guess, we can make it virtual when we need it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It would be possible. Keep in mind that run will only get called on entry from C++ into TorchScript. Function calls go directly through the getPlan pathway. So anything that has to happen for every function call will need to be expressed as part of getPlan.

Expose ExecutionPlan in prep for function calls
Summary: This reorganizes GraphExecutor into a stage that looks up
the execution plan getPlanFor, and a stage that runs the plan. This
separation is necessary for first-class functions since the interpreter
will call getPlanFor to look up the Code for the function it will be
calling rather than recursively calling run on the graph executor.

Test Plan: test_jit.py

gh-metadata: pytorch pytorch 21273 gh/zdevito/47/head
@ezyang ezyang added facebook and removed facebook labels Jun 5, 2019
Expose ExecutionPlan in prep for function calls
Summary: This reorganizes GraphExecutor into a stage that looks up
the execution plan getPlanFor, and a stage that runs the plan. This
separation is necessary for first-class functions since the interpreter
will call getPlanFor to look up the Code for the function it will be
calling rather than recursively calling run on the graph executor.

Test Plan: test_jit.py

gh-metadata: pytorch pytorch 21273 gh/zdevito/47/head
Zachary DeVito and others added 2 commits June 7, 2019 10:28
Expose ExecutionPlan in prep for function calls
Summary: This reorganizes GraphExecutor into a stage that looks up
the execution plan getPlanFor, and a stage that runs the plan. This
separation is necessary for first-class functions since the interpreter
will call getPlanFor to look up the Code for the function it will be
calling rather than recursively calling run on the graph executor.

Test Plan: test_jit.py

gh-metadata: pytorch pytorch 21273 gh/zdevito/47/head
Expose ExecutionPlan in prep for function calls
Summary: This reorganizes GraphExecutor into a stage that looks up
the execution plan getPlanFor, and a stage that runs the plan. This
separation is necessary for first-class functions since the interpreter
will call getPlanFor to look up the Code for the function it will be
calling rather than recursively calling run on the graph executor.

Test Plan: test_jit.py

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

@zdevito merged this pull request in 1517ff6.

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.

6 participants