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

Expression Changes #99

Closed
cjdrake opened this Issue Sep 21, 2014 · 0 comments

Comments

Projects
None yet
1 participant
@cjdrake
Owner

cjdrake commented Sep 21, 2014

Some thoughts on the current state of expressions.

The _ArgumentContainer base class doesn't make much sense. The expression types should be broken down into those expressions with fixed arguments, and n-ary arguments. The Not, Implies, and ITE operators should just have base class Expression, and all of the n-ary operators should have base class _NaryOperator.

The inverted forms (nor, nand, xnor, unequal) are a bit silly. There isn't any reason having them in an expression tree is superior to just wrapping the or/and/xor/equal with a NOT operator. I think I originally added them b/c there was some nice symmetry with the inverse operator and how factoring works. You will see the .invert().factor() in several places. Probably cheaper to just implement an _invert_factor method.

Related to the previous point, currently the NOT operator automatically eliminates "double negatives". For example, it will convert NOT(XNOR(x)) to just XOR(x). Since ExprNot.__new__ should still be used to automatically invert constants and literals, it still makes some sense to reduce NOT(NOT(x)) to just x.

There is a nice benefit to having expressions automatically cache their inverse. That is, X should always have a reference to Not(x).

Simplification needs some work too. It will make the code much cleaner if we stop auto-simplifying trivial argument lists. For example, Or(), and Or(x). Get rid of all the fancy __new__ logic, and just implement all of those simplifications at the end of the simplify method.

  • Rearrange Expression base classes, s/_ArgumentContainer/_NaryOperator
  • Get rid of inverted forms
  • Have expression instances automatically cache their inverse
  • Eliminate auto-simplification of degenerate N-ary operators.

@cjdrake cjdrake added the Change label Sep 21, 2014

@cjdrake cjdrake closed this Sep 27, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment