Skip to content

Reduce algebra canonicalization footprint#2290

Open
ricardoV94 wants to merge 3 commits into
pymc-devs:mainfrom
ricardoV94:less_redundant_rewrites
Open

Reduce algebra canonicalization footprint#2290
ricardoV94 wants to merge 3 commits into
pymc-devs:mainfrom
ricardoV94:less_redundant_rewrites

Conversation

@ricardoV94

Copy link
Copy Markdown
Member
  • Handle the absorbing element in AlgebraicCanonizer
  • Emit canonical forms directly from local_neg_to_mul
  • Speedup AlgebraicCanonizer

`AlgebraicCanonizer` already drops the *neutral* element of `main`
(`mul_calculate` returns [] for 1, `add_calculate` for 0). But `mul` also has
an *absorbing* element that `add` does not, and the canonizer did not know
about it: given `mul(0, x, y)`, `simplify_constants` folds the constant subset
to `0` and re-emits `mul(0, x, y)` with the non-constant factors intact.

`local_mul_zero` existed only to perform the collapse the canonizer skipped,
and it does so less thoroughly -- it scans direct inputs, so it misses
`mul(2, x, 0.5, 0)`, where the zero only emerges once the constants are folded
together.

Teach the canonizer the absorbing element instead:

- new `absorbing_element` argument (default None; `add` keeps None)
- `transform` folds a `main` node with a constant absorbing input directly,
  skipping the `get_num_denum` walk
- `simplify_constants` drops the surviving factors when the constants fold to
  the absorbing element -- ahead of the single-constant shortcut, which would
  otherwise hand back the original num/denum untouched

`local_mul_zero` is therefore no longer registered in canonicalize, which was
its only registration. The rewrite itself is kept for rewrite databases that
run without the canonizer.
`local_neg_to_mul` rewrote every `neg(x)` to `mul(-1, x)`, which by
construction is a fresh `Mul` node for `local_mul_canonizer` to flatten and
re-fold on the next canonicalize iteration. Emit the already-canonical form
instead:

    neg(neg(x))      -> x
    neg(mul(c, *xs)) -> mul(-c, *xs)     (c constant; no extra factor)
    neg(mul(*xs))    -> mul(-1, *xs)     (flat, not mul(-1, mul(*xs)))
    neg(c)           -> -c

With the double-negation case handled here, the `useless_neg_of_neg` pattern
rewriter is redundant in canonicalize, so it is no longer registered there. It
stays registered in specialize, where `local_neg_to_mul` does not run.

Measured over the PyMC model catalogue (98 model/axis compiles): final graphs
are unchanged, while `local_mul_canonizer` loses 31 fires, MergeOptimizer 82,
and `constant_folding` 56.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant