Skip to content

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ricardoV94
Copy link
Member

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:

import pytensor
import pytensor.tensor as pt

x = pt.vector("x")
y = pt.log(pt.sum(pt.exp(x)))

pytensor.config.optimizer_interactive = True
pytensor.config.optimizer_interactive_skip_rewrites = "MergeOptimizer, constant_folding"
pytensor.function([x], y)

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!

Comment on lines +237 to +240
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])
Copy link
Member

Choose a reason for hiding this comment

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

is this faster?

Suggested change
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))

Copy link
Member Author

@ricardoV94 ricardoV94 Jan 9, 2023

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.

Comment on lines +250 to +253
if print_topo_order:
topo_orders.extend([topo for item in obj.outputs])
else:
topo_orders.extend([None for item in obj.outputs])
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
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))

@ricardoV94 ricardoV94 force-pushed the optimizer_verbose_linediff branch from e74167f to b3e2f9b Compare January 9, 2023 14:13
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.

2 participants