Skip to content
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

Add inner graph optimizer and merging capabilities #824

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

brandonwillard
Copy link
Member

@brandonwillard brandonwillard commented Feb 17, 2022

This PR is a replacement for #792 that uses a new inner-graph-tracking Feature and an inner-graph-optimizing GlobalOptimizer to handle inner-graph optimization.

Some important changes included in this PR:

  • Function's can now be constructed from FunctionGraphs without unnecessarily cloning them
  • Added FunctionGraph methods for removing nodes, inputs, and outputs
  • Scan's inner-graphs are now exclusively comprised of FunctionGraphs
  • An InnerGraphOptimizer has been added and made the default outer-most Rewriter in aesara.compile.mode.optdb (i.e. it runs first and orchestrates all the subsequent rewrites). This GlobalOptimizer finds all the inner-graphs and applies rewrites to those, as well as the outermost FunctionGraph, of course. Doing this allows us to perform all the necessary rewrites only within the optimization stage of Function compilation, instead of during the Op.make_thunk or Op.perform like Scan and OpFromGraph currently do. Since Op.make_thunk can be called more than once for the same Op/inner-graph, this change means that inner-graphs only ever need to be rewritten/optimized once, and it preserves the meaning/intention of Op.make_thunk by preventing it from unnecessarily overlapping with the task of graph optimization.

Currently, the biggest issue I have with these changes is that the inner-graph Ops have their inner-FunctionGraphs rewritten in-place, which means that use of these Ops after graph compilation may be confusing or cause issues since the inner-graphs now contain new elements. Some of those new elements may include other Ops that—for instance—do not have gradient implementations (e.g. via "specialization" rewrites and the like).

We could likely avoid these issues by simply cloning the graphs and replacing their Ops within InnerGraphOptimizer, though.

Inner-graph cloning has been added, but it is only used when a Function is created—mostly to avoid unnecessary deep cloning in all other situations.

  • Handle inner-graph merging. This can be done in a follow-up PR, but I have a compound FunctionGraph implementation that might solve this easily.
  • Reset iteration counters in EquillibriumOptimizers between inner-graph runs.

@brandonwillard brandonwillard added enhancement New feature or request important Scan Involves the `Scan` `Op` performance concern labels Feb 17, 2022
@brandonwillard brandonwillard self-assigned this Feb 17, 2022
@brandonwillard brandonwillard force-pushed the add-inner-graph-optimizer branch 8 times, most recently from 31c7ea0 to d81c058 Compare February 20, 2022 01:18
@codecov
Copy link

codecov bot commented Feb 20, 2022

Codecov Report

Merging #824 (9505933) into main (c433071) will increase coverage by 3.67%.
The diff coverage is 86.31%.

❗ Current head 9505933 differs from pull request most recent head e624896. Consider uploading reports for the commit e624896 to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #824      +/-   ##
==========================================
+ Coverage   74.86%   78.54%   +3.67%     
==========================================
  Files         194      152      -42     
  Lines       50107    47911    -2196     
  Branches    12098    10882    -1216     
==========================================
+ Hits        37514    37630     +116     
+ Misses      10266     7736    -2530     
- Partials     2327     2545     +218     
Impacted Files Coverage Δ
aesara/compile/io.py 83.01% <0.00%> (ø)
aesara/printing.py 48.58% <0.00%> (-2.96%) ⬇️
aesara/scan/basic.py 86.57% <ø> (+1.98%) ⬆️
aesara/compile/debugmode.py 60.45% <50.00%> (-0.83%) ⬇️
aesara/scan/opt.py 80.90% <68.08%> (+80.90%) ⬆️
aesara/gradient.py 77.23% <71.42%> (+0.20%) ⬆️
aesara/compile/function/pfunc.py 79.89% <75.00%> (-4.29%) ⬇️
aesara/link/c/basic.py 87.36% <75.00%> (+0.22%) ⬆️
aesara/compile/function/types.py 79.10% <79.74%> (-0.54%) ⬇️
aesara/graph/destroyhandler.py 69.55% <80.00%> (-0.39%) ⬇️
... and 193 more

@brandonwillard brandonwillard force-pushed the add-inner-graph-optimizer branch 9 times, most recently from 5d1590f to 7300338 Compare February 21, 2022 21:43
@brandonwillard brandonwillard changed the title Add inner graph optimizer Add inner graph optimizer and merging capabilities Feb 22, 2022
@brandonwillard brandonwillard force-pushed the add-inner-graph-optimizer branch 4 times, most recently from ac20385 to 8d1334f Compare February 26, 2022 21:44
This new `InnerGraphOptimizer` is now the default outer-most optimizer in
`optdb` (i.e. Aesara's entry point to optimization).  This `GlobalOptimizer`
collects inner-`FunctionGraph`s inside its `FunctionGraph` target and performs
all the same rewrites on each `FunctionGraph`, as well as new
inner-`FunctionGraph`s created while rewriting existing ones.  It is intended to
serve as the only means of inner-graph optimization; that is why `OpFromGraph`
and `Scan` now have their inner-graph compilation `Mode`s cloned with
`optimizer=None`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request important performance concern Scan Involves the `Scan` `Op`
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Merge Scan's inner-graphs without graph traversal
1 participant