rust-or/rust-lp-modeler

Simplify the expression tree

Opened this issue · 2 comments

lovasoa commented

What would you think about simplifying the expression tree by

  • Removing SubExpr (and replacing it with Add(Mul(-1, expr))
  • Replacing AddExpr(left, right) by AddExpr(Vec<expr>), to avoid creating deep trees (that cause #37)

This is not perfect, but should be easy to implement as a workaround for now. Would you accept such a PR ?

A more ambitious refactor that we can keep for later is to implement the expression as a simple HashMap<Variable, Coefficient>.

jcavat commented

is it still relevant after last commits ?

lovasoa commented

removing Subexpr would significantly simplify the code.
And restructuring the struct to have only a sum of multiplications would improve performance

  • 👍1