Skip to content

Separate forward and backwad compilation and support higher order derivatives for aot_function #856

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

Open
wants to merge 12 commits into
base: gh/anjali411/1/base
Choose a base branch
from

Conversation

anjali411
Copy link

@anjali411 anjali411 commented Jun 7, 2022

Stack from ghstack (oldest at bottom):

Test Plan: Existing tests should pass

anjali411 added a commit that referenced this pull request Jun 7, 2022
@anjali411 anjali411 requested review from Chillee and zou3519 June 7, 2022 19:26
anjali411 added a commit that referenced this pull request Jun 7, 2022
…tion"


Test Plan: Existing tests should pass

[ghstack-poisoned]
anjali411 added a commit that referenced this pull request Jun 7, 2022
@anjali411 anjali411 removed request for zou3519 and Chillee June 7, 2022 20:36
@@ -140,12 +140,15 @@ def create_aot_autograd_function(
compiled_fw = None
compiled_bw = None
num_outs = None
joint_inputs = None
Copy link
Author

Choose a reason for hiding this comment

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

need to save these tensors in the context

…tion"


Test Plan: Existing tests should pass

[ghstack-poisoned]
anjali411 added a commit that referenced this pull request Jun 9, 2022
Comment on lines 217 to 220
func_code = bw_module.code.split('self, ')
# print(func_code[0] + func_code[1])
exec(func_code[0] + func_code[1], globals())
f = create_aot_autograd_function(forward, bw_compiler, bw_compiler, partition_fn, aot_decompositions, grad_state)
Copy link
Contributor

Choose a reason for hiding this comment

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

Two questions:

  • Why are we passing forward to create_aot_autograd_function? I would have expected us to pass bw_module.code without the self argument
  • What is the exec for? Are you trying to test this without the create_aot_autograd_function line?

Copy link
Author

Choose a reason for hiding this comment

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

  • forward is the name of the function generated by running bw_module.code
  • exec executes the bw_module.code to create a backward function which is the forward for the next pass

…tion"


Test Plan: Existing tests should pass

[ghstack-poisoned]
anjali411 added a commit that referenced this pull request Jun 14, 2022
…tion"


Test Plan: Existing tests should pass

[ghstack-poisoned]
anjali411 added a commit that referenced this pull request Jun 21, 2022
ghstack-source-id: 056ba4b
Pull Request resolved: #856
…tion"


Test Plan: Existing tests should pass

[ghstack-poisoned]
anjali411 added a commit that referenced this pull request Jun 21, 2022
ghstack-source-id: 9f2c626
Pull Request resolved: #856
…tion"


Test Plan: Existing tests should pass

[ghstack-poisoned]
anjali411 added a commit that referenced this pull request Jul 12, 2022
ghstack-source-id: 0ce1d4a
Pull Request resolved: #856
@anjali411 anjali411 requested a review from Chillee July 13, 2022 15:27
…tion"


Test Plan: Existing tests should pass

[ghstack-poisoned]
anjali411 added a commit that referenced this pull request Jul 13, 2022
ghstack-source-id: f154dd1
Pull Request resolved: #856
@anjali411 anjali411 changed the title Separate forward and backwad compilation for default partition Separate forward and backwad compilation and support higher order derivates for aot_function Jul 13, 2022
@anjali411 anjali411 changed the title Separate forward and backwad compilation and support higher order derivates for aot_function Separate forward and backwad compilation and support higher order derivatives for aot_function Jul 13, 2022
…r order derivatives for aot_function"


Test Plan: Existing tests should pass

[ghstack-poisoned]
anjali411 added a commit that referenced this pull request Jul 13, 2022
ghstack-source-id: 6575ea0
Pull Request resolved: #856
…r order derivatives for aot_function"


Test Plan: Existing tests should pass

[ghstack-poisoned]
anjali411 added a commit that referenced this pull request Jul 13, 2022
ghstack-source-id: a215569
Pull Request resolved: #856
…r order derivatives for aot_function"


Test Plan: Existing tests should pass

[ghstack-poisoned]
anjali411 added a commit that referenced this pull request Jul 13, 2022
ghstack-source-id: 0b78895
Pull Request resolved: #856
def fake_fn(primals, tangents):
return fx_g_b(primals, tangents)
fx_g_b = make_fx(functionalize(fake_fn))(flat_tensor_args, inp_grad_outs)
saved_value_nodes = _get_saved_values(fx_g_b, saved_value_names)
Copy link
Author

Choose a reason for hiding this comment

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

Unfortunately this approach doesn't always work because the newly generated fx graph may not have the same nodes as the previous graph. We need an alternate way to select nodes of interest in this new graph!

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.

3 participants