-
Notifications
You must be signed in to change notification settings - Fork 123
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
Associate fx node as origins on IRNode, ensure all nodes going to scheduling have an associate FX Node #754
Conversation
@@ -201,7 +201,13 @@ def is_triton(device): | |||
|
|||
|
|||
class IRNode(object): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we just put everything on the IRNode base class? Having things scattered on specific node constructors doesn't seem great.
Something like:
class IRNode:
_current_origins = set()
@staticmethod
@contexlib.contextmanager
def current_origin(origins: Set[fx.Node])
old = IRNode._current_origins
IRNode._current_origins = old | origins
yield
IRNode._current_origins = old
def __init__(self):
self.origins = set(self._current_origins)
Then we could just throw a:
with IRNode.current_origins({n}):
...
Around the entire body of run_node(n)
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I thought about it, but I wasn't sure it was right for all IRNode to have this property on it. Is there any case where a subclass of IRNode having an origins
is nonsensical?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, every node can have an origins.
torchinductor/lowering.py
Outdated
@@ -28,6 +28,8 @@ | |||
prims = torch.ops.prims | |||
needs_realized_inputs = set() | |||
|
|||
current_origin = None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unused?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes
An example of what these IRNodes look like now, running
test_alexnet_prefix_cpu