Add kwarg 'ignore' to cse() for disqualification. #12010

Open
wants to merge 2 commits into
from

Projects

None yet

3 participants

@bjodah
Member
bjodah commented Jan 2, 2017

I needed a way to tell cse not to create substitutions containing a subset of symbols.
I've implemented this functionality by adding a keyword argument ignore to cse.

I found this to be useful e.g. when doing code-generation where a set of variables may change much more often than others. If you don't include the frequently changing variables in the common subexpressions, then the substitutions stay valid over e.g. iterations in numerical code.

Let me know what you think.

@bjodah
Member
bjodah commented Jan 10, 2017

cc @cdsousa & @smichr if interested (based on git blame).

sympy/simplify/tests/test_cse.py
+ if y in sub.free_symbols:
+ break # expected behaviour
+ else:
+ raise ValueError("cse failed to identify any term with y")
@asmeurer
asmeurer Jan 10, 2017 Member

It would be better to use AssertionError here. In the loop below you can just write assert y not in sub.free_symbols, "Sub-expressions containing y must be ignored". You could also rewrite this as

assert any(y in sub.free_symbols for symb, sub in subst1), "cse failed to identify any term with y"
@asmeurer
Member

Just the one comment. +1 otherwise. Feel free to request reviews (using the GitHub feature) from me for codegen related PRs.

@asmeurer asmeurer self-requested a review Jan 10, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment