add_subexpressions_for_constants and new_filtered fix

Two small patches:

• Added a function add_subexpressions_for_constants to pystencils.simp. This function extracts numerical constants like 2 or 1/3 from equations. This is helpful because SymPy does not exclude common factors from sums if they are not symbols. Excluding them this way can reduce the number of multiplications. In some cases, additional common subexpressions can also be found.
• Changed AssignmentCollection.new_filtered to use self.copy to preserve other members of the assignment collection.

Can you provide a test case for factoring out the constants?

Done.

Wouldn't it make sense to formulate some of those concepts in terms of sympy.codegen.rewriting.optimize.ReplaceOptim?

E.g. we have

    # Evaluates all constant terms
    evaluate_constant_terms = ReplaceOptim(
        lambda e: hasattr(e, 'is_constant') and e.is_constant and not e.is_integer,
        lambda p: p.evalf()
    )

in pystencils.math_optimizations. This approach does not have the advantage of creating subexpressions automatically. But you could just collect the subexpressions to be created in the replace-lambda above. ReplaceOptim has to arguments: one where to replace something and second to replace by what. Can be applied also on ASTs and AssignmentCollections. The advantage of using Sympy's optimization classes is that a user can just select the optimization the they want and call pystencils.math_optimization.optimize_ast(ast, [list_of, optimizations, and_even_more]) or optimize(expr, [list_of, optimizations, and_even_more]).

Sympy has also some clever although few predefined optimizations. E.g. it can replace exp(x - 1) by expm1(x)

I am not sure it this is feasbile in this case. I think we pretty much need the subexpressions since we want to add in the real value of the symbol in the end, right?

but you can collect sub-expressions in the lambda of the ReplaceOptim. You create a subexpression and replace with the lhs of this new subexpression in the original term.

The other optimization replaces the numerical values by evaluating to a float. So 2/3 by 0.6666666

Those sympy optimizations are certainly a nice feature, but I think they are not fit for this purpose. sympy.codegen.rewriting.Optimization and its subclasses strike me as a means of doing local optimizations, while add_subexpressions_for_constants is explicitly affecting an AssignmentCollection globally. It should also only be applied to assignment collections (because where to put the subexpressions, otherwise?), but using ReplaceOptim would drop that restriction. However, I think the recursive sub-function can be removed in favor of sympy.replace.

I would agree that the sympy optimizations do not really fit in this case.

added 1 commit

• 0baa6f61 - Added Test Case. Now using sp.replace.

Compare with previous version

added 1 commit

• eb850beb - Corrected recursion order to catch coarsest constant expression possible

Compare with previous version

added 1 commit

• 68b1efe2 - Reverted previous changes because they caused unexprected problems with exponentials.

Compare with previous version

merged

mentioned in commit b2312d53