Improve normalization with nested functions.

normal() fails to fully normalize expressions where nested functions occur
multiple times because replace_with_symbol() searches repl for an
original expression but inserts into repl the substitued expression. Then
the repeated search does not work and a new symbol is introduced.

This patch fixes that by searching for the expression with the repl
substituted instead of for the original expression.

diff --git a/check/exam_normalization.cpp b/check/exam_normalization.cpp
index a184ad28ecd2d030e686d34bc36120efe5045b69..d98dec681a79d9071d03ccc3cfd300f36545cdcc 100644 (file)
--- a/check/exam_normalization.cpp
+++ b/check/exam_normalization.cpp
@@ -163,7 +163,12 @@ static unsigned exam_normal4()
        e = (pow(x-y*2,4)/pow(pow(x,2)-pow(y,2)*4,2)+1)*(x+y*2)*(y+z)/(pow(x,2)+pow(y,2)*4);
        d = (y*2 + z*2) / (x + y*2);
        result += check_normal(e, d);
-       
+
+       // Replacement of nested functions with temporary symbols
+       e = x/(sqrt(sin(z)-1)) + y/(sqrt(sin(z)-1));
+       d = (x + y)/(sqrt(sin(z)-1));
+       result += check_normal(e, d);
+
        return result;
 }
 

diff --git a/ginac/normal.cpp b/ginac/normal.cpp
index 97bd156ae06a2972c9b4dcb1d117b67d2a143eec..f22bf27b131ec1952b6f0f4192da37b842e51ba9 100644 (file)
--- a/ginac/normal.cpp
+++ b/ginac/normal.cpp
@@ -2006,16 +2006,18 @@ ex sqrfree_parfrac(const ex & a, const symbol & x)
  *  @see ex::normal */
 static ex replace_with_symbol(const ex & e, exmap & repl, exmap & rev_lookup)
 {
+       // Since the repl contains replaced expressions we should search for them
+       ex e_replaced = e.subs(repl, subs_options::no_pattern);
+
        // Expression already replaced? Then return the assigned symbol
-       exmap::const_iterator it = rev_lookup.find(e);
+       exmap::const_iterator it = rev_lookup.find(e_replaced);
        if (it != rev_lookup.end())
                return it->second;
-       
+
        // Otherwise create new symbol and add to list, taking care that the
        // replacement expression doesn't itself contain symbols from repl,
        // because subs() is not recursive
        ex es = (new symbol)->setflag(status_flags::dynallocated);
-       ex e_replaced = e.subs(repl, subs_options::no_pattern);
        repl.insert(std::make_pair(es, e_replaced));
        rev_lookup.insert(std::make_pair(e_replaced, es));
        return es;
@@ -2028,16 +2030,18 @@ static ex replace_with_symbol(const ex & e, exmap & repl, exmap & rev_lookup)
  *  @see basic::to_polynomial */
 static ex replace_with_symbol(const ex & e, exmap & repl)
 {
+       // Since the repl contains replaced expressions we should search for them
+       ex e_replaced = e.subs(repl, subs_options::no_pattern);
+
        // Expression already replaced? Then return the assigned symbol
        for (exmap::const_iterator it = repl.begin(); it != repl.end(); ++it)
-               if (it->second.is_equal(e))
+               if (it->second.is_equal(e_replaced))
                        return it->first;
-       
+
        // Otherwise create new symbol and add to list, taking care that the
        // replacement expression doesn't itself contain symbols from repl,
        // because subs() is not recursive
        ex es = (new symbol)->setflag(status_flags::dynallocated);
-       ex e_replaced = e.subs(repl, subs_options::no_pattern);
        repl.insert(std::make_pair(es, e_replaced));
        return es;
 }