-
Notifications
You must be signed in to change notification settings - Fork 129
Add interactive optimization mode #187
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
base: main
Are you sure you want to change the base?
Conversation
if print_topo_order: | ||
topo_orders.extend([topo for item in obj.maker.fgraph.outputs]) | ||
else: | ||
topo_orders.extend([None for item in obj.maker.fgraph.outputs]) |
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.
is this faster?
if print_topo_order: | |
topo_orders.extend([topo for item in obj.maker.fgraph.outputs]) | |
else: | |
topo_orders.extend([None for item in obj.maker.fgraph.outputs]) | |
if print_topo_order: | |
topo_orders.extend([topo] * len(obj.maker.fgraph.outputs)) | |
else: | |
topo_orders.extend([None] * len(obj.maker.fgraph.outputs)) |
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.
It's 2x faster (with 5 elements), but the rest of this function uses the list comprehension approach. So I would either change them all or keep as is.
if print_topo_order: | ||
topo_orders.extend([topo for item in obj.outputs]) | ||
else: | ||
topo_orders.extend([None for item in obj.outputs]) |
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.
if print_topo_order: | |
topo_orders.extend([topo for item in obj.outputs]) | |
else: | |
topo_orders.extend([None for item in obj.outputs]) | |
if print_topo_order: | |
topo_orders.extend([topo] * len(obj.maker.fgraph.outputs)) | |
else: | |
topo_orders.extend([None] * len(obj.maker.fgraph.outputs)) |
e74167f
to
b3e2f9b
Compare
I have resorted to patching the optimizer verbose to print the FunctionGraph before and after intermediate changes, so I thought about making this a builtin option. If you want to see how it looks locally, you can try this snippet:
I quite like it, except for the fact that the
Differ.compare
returns results line-by-line, when it would sometimes be much more readable if the additions/deletions from the same "block" were grouped together. If anyone would like to take a stab at grouping the differences that would be great!