Skip to content

Conversation

@izdeby
Copy link
Contributor

@izdeby izdeby commented Dec 9, 2019

This stack is a first step toward an effort to fix, clean up and simplify code generation logic. 
Please see the master task to see related discussions and all the known issues.

Main focus of these changes is TensorOptions in code generation.
Goals:

  • Remove TensorOptions from generated code wherever it's possible. Leave it only in python/C++ API layers.
  • Refactor TensorOptions logic to a single place.
  • Log all discovered issues.

Non goals:

  • Fix Everything!
  • Remove all the hacks in code generation scripts.
  • Clean up and defector all code generation scripts.

In this PR:
Add tracing support for optional Device and Layout types.


Stack from ghstack:

Differential Revision: D18912685

@izdeby izdeby requested a review from apaszke as a code owner December 9, 2019 18:54
@facebook-github-bot facebook-github-bot added the oncall: jit Add this issue/PR to JIT oncall triage queue label Dec 9, 2019
@izdeby izdeby changed the title Add tracing support for optional Device and Layout [WIP] Add tracing support for optional Device and Layout Dec 9, 2019
@izdeby izdeby changed the title [WIP] Add tracing support for optional Device and Layout Add tracing support for optional Device and Layout Dec 9, 2019
@gchanan gchanan requested a review from eellison December 9, 2019 23:47
@gchanan
Copy link
Contributor

gchanan commented Dec 9, 2019

Someone from JIT (e.g. @eellison should review this).

Copy link
Contributor

@eellison eellison left a comment

Choose a reason for hiding this comment

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

LGTM. I don't know how exactly you plan on landing this stack; could you not land this PR without adding tests for these changes ? If it's in an upcoming PR that's fine.

if (value.has_value()) {
detail::genericAddInput(n, static_cast<int64_t>(*value));
} else {
Graph* g = n->owningGraph();
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be nice if you factored the 8 copy-pasta's of this three-line block into it's own function (i am one of those copy-paster's...)

This stack is a first step toward an effort to fix, clean up and simplify code generation logic. 
Please see the master [task](#30405) to see related discussions and all the known issues.

Main focus of these changes is TensorOptions in code generation. 
Goals:
- Remove TensorOptions from generated code wherever it's possible. Leave it only in python/C++ API layers.
- Refactor TensorOptions logic to a single place.
- Log all discovered issues.

Non goals:
- Fix Everything! 
- Remove all the hacks in code generation scripts.
- Clean up and defector all code generation scripts.

--------------
In this PR: 
Add tracing support for optional Device and Layout types.

--------------




[ghstack-poisoned]
This stack is a first step toward an effort to fix, clean up and simplify code generation logic. 
Please see the master [task](#30405) to see related discussions and all the known issues.

Main focus of these changes is TensorOptions in code generation. 
Goals:
- Remove TensorOptions from generated code wherever it's possible. Leave it only in python/C++ API layers.
- Refactor TensorOptions logic to a single place.
- Log all discovered issues.

Non goals:
- Fix Everything! 
- Remove all the hacks in code generation scripts.
- Clean up and defector all code generation scripts.

--------------
In this PR: 
Add tracing support for optional Device and Layout types.

--------------


Differential Revision: [D18912685](https://our.internmc.facebook.com/intern/diff/D18912685)

[ghstack-poisoned]
@facebook-github-bot
Copy link
Contributor

@izdeby merged this pull request in 44ecc3a.

@facebook-github-bot facebook-github-bot deleted the gh/izdeby/29/head branch December 15, 2019 15:16
wuhuikx pushed a commit to wuhuikx/pytorch that referenced this pull request Jan 30, 2020
Summary:
Pull Request resolved: pytorch#30979

This stack is a first step toward an effort to fix, clean up and simplify code generation logic. �Please see the master [task](pytorch#30405) to see related discussions and all the known issues.

Main focus of these changes is TensorOptions in code generation.
Goals:
- Remove TensorOptions from generated code wherever it's possible. Leave it only in python/C++ API layers.
- Refactor TensorOptions logic to a single place.
- Log all discovered issues.

Non goals:
- Fix Everything!
- Remove all the hacks in code generation scripts.
- Clean up and defector all code generation scripts.

--------------
In this PR:
Add tracing support for optional Device and Layout types.

--------------

Test Plan: Imported from OSS

Differential Revision: D18912685

Pulled By: izdeby

fbshipit-source-id: 4a9514ce2eee0041f9bc96636d3ddb4f077675e1
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