fixes in nex order, add nex_mul::m_coeff

Signed-off-by: Lev Nachmanson <levnach@hotmail.com>

src/math/lp/cross_nested.h

@@ -257,6 +257,7 @@ public:
                 TRACE("nla_cn", tout << "got the cn form: =" << *m_e << "\n";);
                 m_done = m_call_on_result(m_e) || ++m_reported > 100;
  #ifdef Z3DEBUG
+                TRACE("nla_cn", tout << "m_e_clone" << *m_e_clone << "\n";);
                 SASSERT(nex_creator::equal(m_e, m_e_clone));
  #endif
             } else {

src/math/lp/nex.h

@@ -164,9 +164,19 @@ public:
 
 
 class nex_mul : public nex {
+    rational        m_coeff;
     vector<nex_pow> m_children;
 public:
-    nex_mul()  {}
+    nex_mul() : m_coeff(rational(1)) {}
+
+    const rational& coeff() const {
+        return m_coeff;
+    }
+
+    rational& coeff() {
+        return m_coeff;
+    }
+    
     unsigned size() const { return m_children.size(); }
     expr_type type() const { return expr_type::MUL; }
     vector<nex_pow>& children() { return m_children;}
@@ -176,6 +186,10 @@ public:
     std::ostream & print(std::ostream& out) const {
         
         bool first = true;
+        if (!m_coeff.is_one()) {
+            out << m_coeff;
+            first = false;
+        }
         for (const nex_pow& v : m_children) {            
             std::string s = v.to_string();
             if (first) {
@@ -189,18 +203,13 @@ public:
     }
 
     void add_child(nex* e) {
+        if (e->is_scalar()) {
+            m_coeff *= to_scalar(e)->value();
+            return;
+        }
         add_child_in_power(e, 1);
     }
 
-    // returns true if the product of scalars gives a number different from 1
-    bool has_a_coeff() const {
-        rational r(1);
-        for (auto & p : *this) {
-            if (p.e()->is_scalar())
-                r *= to_scalar(p.e())->value();
-        }
-        return !r.is_one();
-    }
 
     const nex_pow& operator[](unsigned j) const { return m_children[j]; }
     nex_pow& operator[](unsigned j) { return m_children[j]; }
@@ -209,7 +218,13 @@ public:
     nex_pow* begin() { return m_children.begin(); }
     nex_pow* end() { return m_children.end(); }
     
-    void add_child_in_power(nex* e, int power) { m_children.push_back(nex_pow(e, power)); }
+    void add_child_in_power(nex* e, int power) {        
+        if (e->is_scalar()) {
+            m_coeff *= (to_scalar(e)->value()).expt(power);
+            return;
+        }
+        m_children.push_back(nex_pow(e, power));
+    }
 
     bool contains(lpvar j) const {
         for (const nex_pow& c : *this) {

src/math/lp/nex_creator.cpp

@@ -30,7 +30,8 @@ nex * nex_creator::mk_div(const nex* a, lpvar j) {
         return mk_scalar(rational(1));
     vector<nex_pow> bv; 
     bool seenj = false;
-    for (auto& p : *to_mul(a)) {
+    auto ma = to_mul(a);
+    for (auto& p : *ma) {
         const nex * c = p.e();
         int pow = p.pow();
         if (!seenj && c->contains(j)) {
@@ -50,41 +51,37 @@ nex * nex_creator::mk_div(const nex* a, lpvar j) {
             bv.push_back(nex_pow(clone(c), pow));
         }
     }
-    if (bv.size() > 1) { 
+    if (bv.size() == 1 && bv.begin()->pow() == 1 && ma->coeff().is_one()) {
-        return mk_mul(bv);
-    }
-    if (bv.size() == 1 && bv.begin()->pow() == 1) {
         return bv.begin()->e();
     }
-    if (bv.size() == 0)
+    if (bv.size() == 0) {
-        return mk_scalar(rational(1));
+        return mk_scalar(rational(ma->coeff()));
-    return mk_mul(bv);
+    }
+    
+    auto m = mk_mul(bv);
+    m->coeff() = ma->coeff();
+    return m;
+
 }
 
-bool nex_creator::eat_scalar_pow(nex_scalar *& r, nex_pow& p, unsigned pow) {
+bool nex_creator::eat_scalar_pow(rational& r, const nex_pow& p, unsigned pow) {
     if (!p.e()->is_scalar())
         return false;
-    nex_scalar *pe = to_scalar(p.e());
+    const nex_scalar *pe = to_scalar(p.e());
     if (pe->value().is_one())
-        return true; // but do not change r here
+        return true; // r does not change here
-    if (r == nullptr) {
+    r *= pe->value().expt(p.pow() * pow);
-        r = pe;
-        r->value() = r->value().expt(p.pow()*pow);                
-    } else {
-        r->value() *= pe->value().expt(p.pow()*pow);
-    }
     return true;
 }
 
 
-void nex_creator::simplify_children_of_mul(vector<nex_pow> & children) {
+void nex_creator::simplify_children_of_mul(vector<nex_pow> & children, rational& coeff) {
-    nex_scalar* r = nullptr;
     TRACE("nla_cn_details", print_vector(children, tout););
     vector<nex_pow> to_promote;
     int skipped = 0;
     for(unsigned j = 0; j < children.size(); j++) {        
         nex_pow& p = children[j];
-        if (eat_scalar_pow(r, p, 1)) {
+        if (eat_scalar_pow(coeff, p, 1)) {
             skipped++;
             continue;
         }
@@ -104,25 +101,21 @@ void nex_creator::simplify_children_of_mul(vector<nex_pow> & children) {
 
     for (nex_pow & p : to_promote) {
         TRACE("nla_cn_details", tout << p << "\n";);
-        for (nex_pow& pp : *to_mul(p.e())) {
+        nex_mul *pm = to_mul(p.e());
+        for (nex_pow& pp : *pm) {
             TRACE("nla_cn_details", tout << pp << "\n";);
-            if (!eat_scalar_pow(r, pp, p.pow()))
+            if (!eat_scalar_pow(coeff, pp, p.pow()))
                 children.push_back(nex_pow(pp.e(), pp.pow() * p.pow()));            
         }
+        coeff *= pm->coeff().expt(p.pow());
     }
 
-    if (r != nullptr) {
-        children.push_back(nex_pow(r));
-    }
-        
     mul_to_powers(children);
     
     TRACE("nla_cn_details", print_vector(children, tout););    
 }
 
-bool nex_creator::less_than_on_mul(const nex_mul* a, const nex_mul* b) const {
+bool nex_creator::less_than_on_mul_mul(const nex_mul* a, const nex_mul* b) const {
-    // the scalar, if it is there, is at the beginning of the children()
-    TRACE("nla_cn_details", tout << "a = " << *a << ", b = " << *b << "\n";);
     SASSERT(is_simplified(a));
     SASSERT(is_simplified(b));
     unsigned a_deg = a->get_degree();
@@ -163,9 +156,12 @@ bool nex_creator::less_than_on_mul(const nex_mul* a, const nex_mul* b) const {
             inside_a_p = inside_b_p = false;
             it_a++; it_b++;
             if (it_a == a_end) {
-                return it_b != b_end;
+                if (it_b != b_end) {
+                    return true;
+                }
+                return a->coeff() < b->coeff();
             } else if (it_b == b_end) {
-                return true;                
+                return false;                
             }
             // no iterator reached the end
             continue;
@@ -187,6 +183,7 @@ bool nex_creator::less_than_on_mul(const nex_mul* a, const nex_mul* b) const {
             inside_b_p = false;
         }
     }
+    TRACE("nla_cn_details", tout << "a = " << *a << " >= b = " << *b << "\n";);
     return false;
 
 }
@@ -206,7 +203,6 @@ bool nex_creator::less_than_on_var_nex(const nex_var* a, const nex* b) const {
             const nex * f = c.e();
             return less_than_on_var_nex(a, f);
         }
-            
     case expr_type::SUM:
         {
             return !lt((*to_sum(b))[0], a);
@@ -228,10 +224,10 @@ bool nex_creator::less_than_on_mul_nex(const nex_mul* a, const nex* b) const {
             auto it = a->begin();
             const nex_pow & c  = *it;
             const nex * f = c.e();
-            return lt(f, a);
+            return lt(f, b);
         }
     case expr_type::MUL:
-        return less_than_on_mul(a, to_mul(b));            
+        return less_than_on_mul_mul(a, to_mul(b));            
     case expr_type::SUM:
         return lt(a, (*to_sum(b))[0]);
     default:
@@ -253,7 +249,7 @@ bool nex_creator::less_than_on_sum_sum(const nex_sum* a, const nex_sum* b) const
 }
 
 bool nex_creator::lt(const nex* a, const nex* b) const {
-    TRACE("nla_cn_details", tout << *a << " ^ " << *b << "\n";);
+    TRACE("nla_cn_details_", tout << *a << " ? " << *b <<  "\n";);
     bool ret;
     switch (a->type()) {
     case expr_type::VAR: 
@@ -302,8 +298,10 @@ bool nex_creator::is_sorted(const nex_mul* e) const {
 
 
 bool nex_creator::mul_is_simplified(const nex_mul* e) const {
-    TRACE("nla_cn_details", tout <<  "e = " << *e << "\n";);
+    if (e->size() == 0)
-    if (e->size() == 1 && e->begin()->pow() == 1)
+        return false; // it has to be a scalar
+    TRACE("nla_cn_details_", tout <<  "e = " << *e << "\n";);
+    if (e->size() == 1 && e->begin()->pow() == 1 && e->coeff().is_one())
         return false;
     std::set<const nex*, nex_lt> s([this](const nex* a, const nex* b) {return lt(a, b); });
     for (const auto &p : *e) {
@@ -334,8 +332,9 @@ bool nex_creator::mul_is_simplified(const nex_mul* e) const {
 
 nex * nex_creator::simplify_mul(nex_mul *e) {
     TRACE("nla_cn_details", tout << *e << "\n";);
-    simplify_children_of_mul(e->children());
+    rational& coeff = e->coeff();
-    if (e->size() == 1 && (*e)[0].pow() == 1) 
+    simplify_children_of_mul(e->children(), coeff);
+    if (e->size() == 1 && (*e)[0].pow() == 1 && coeff.is_one()) 
         return (*e)[0].e();
     TRACE("nla_cn_details", tout << *e << "\n";);
     SASSERT(is_simplified(e));
@@ -445,41 +444,24 @@ bool nex_creator::register_in_join_map(std::map<nex*, rational, nex_lt>& map, ne
     auto map_it = map.find(e);
     if (map_it == map.end()) {
         map[e] = r;
+        TRACE("nla_cn_details",  tout << "inserting " << std::endl;);
         return false;
     } else {
         map_it->second += r;
+        TRACE("nla_cn_details",  tout << "adding" << r << std::endl;);
         return true;
     }
 }
 
 // returns true if a simplificatian happens
-bool nex_creator::process_mul_in_simplify_sum(nex_mul* em, std::map<nex*, rational, nex_lt> &map) {
+bool nex_creator::process_mul_in_simplify_sum(nex_mul* em, std::map<nex*, rational, nex_lt> &map) {    
-    bool found = false;
+    return register_in_join_map(map, em, em->coeff());
-    auto it = em->begin();
-    if (it->e()->is_scalar()) {
-        SASSERT(it->pow() == 1);
-        rational r = to_scalar(it->e())->value();              
-        auto end = em->end();
-        if (em->size() == 2 && (*em)[1].pow() == 1) {
-            found = register_in_join_map(map, (*em)[1].e(), r);
-        } else {
-            nex_mul * m = mk_mul();
-            for (it++; it != end; it++) {
-                m->add_child_in_power(it->e(), it->pow());
-            }
-            found = register_in_join_map(map, m, r);
-        } 
-    } else {
-        found = register_in_join_map(map, em, rational(1));
-    }
-    return found;
 }
 
 bool nex_creator::fill_join_map_for_sum(ptr_vector<nex> & children,
                            std::map<nex*, rational, nex_lt>& map,
                            std::unordered_set<nex*>& existing_nex,
                            nex_scalar*& common_scalar) {
-    
     common_scalar = nullptr;
     bool simplified = false;
     for (auto e : children) {
@@ -621,9 +603,10 @@ nex * nex_creator::mk_div_mul_by_mul(const nex_mul *a, const nex_mul* b) {
     SASSERT(m_powers.size() == 0);
     if (ret->size() == 0) {
         delete ret;
-        TRACE("nla_cn_details", tout << "return 1\n";);
+        TRACE("nla_cn_details", tout << "return scalar\n";);
-        return mk_scalar(rational(1));
+        return mk_scalar(a->coeff() / b->coeff());
     }
+    ret->coeff() = a->coeff() / b->coeff();
     add_to_allocated(ret);
     TRACE("nla_cn_details", tout << *ret << "\n";);        
     return ret;
@@ -636,7 +619,7 @@ nex * nex_creator::mk_div_by_mul(const nex* a, const nex_mul* b) {
     }
     
     if (a->is_var()) {
-        SASSERT(b->get_degree() == 1 && get_vars_of_expr(a) == get_vars_of_expr(b));
+        SASSERT(b->get_degree() == 1 && get_vars_of_expr(a) == get_vars_of_expr(b) && b->coeff().is_one());
         return mk_scalar(rational(1));
     }    
     return mk_div_mul_by_mul(to_mul(a), b);
@@ -676,15 +659,9 @@ void nex_creator::process_map_pair(nex *e, const rational& coeff, ptr_vector<nex
             if (e->is_var()) {
                 children.push_back(mk_mul(mk_scalar(coeff), e));
             } else {
-                SASSERT(e->is_mul());
+                to_mul(e)->coeff() = coeff;
-                nex* first = (*to_mul(e))[0].e();
+                e = simplify(e);
-                if (first->is_scalar()) {
+                children.push_back(e);
-                    to_scalar(first)->value() = coeff;
-                    children.push_back(e);
-                } else {
-                    e = simplify(mk_mul(mk_scalar(coeff), e));
-                    children.push_back(e);
-                }
             }
         }                
     } else { // e is new
@@ -715,6 +692,7 @@ unsigned nex_creator::find_sum_in_mul(const nex_mul* a) const {
     return -1;
 }
 nex* nex_creator::canonize_mul(nex_mul *a) {    
+    TRACE("nla_cn_details", tout << "a = " << *a << "\n";);
     unsigned j = find_sum_in_mul(a);
     if (j + 1 == 0)
         return a;
@@ -736,7 +714,7 @@ nex* nex_creator::canonize_mul(nex_mul *a) {
         }
         r->add_child(m);
     }
-    TRACE("nla_cn_details", tout << *r << "\n";);
+    TRACE("nla_cn_details", tout << "canonized a = " <<  *r << "\n";);
     return canonize(r);
 }
 
@@ -759,6 +737,7 @@ nex* nex_creator::canonize(const nex *a) {
 }
 
 bool nex_creator::equal(const nex* a, const nex* b) {
+    TRACE("nla_cn_details", tout << *a << " against  " << *b << "\n";);
     nex_creator cn;
     unsigned n = 0;
     for (lpvar j : get_vars_of_expr(a)) {

src/math/lp/nex_creator.h

@@ -93,7 +93,7 @@ public:
         return (a.pow() < b.pow()) || (a.pow() == b.pow() && lt(a.e(), b.e()));
     }
     
-    void simplify_children_of_mul(vector<nex_pow> & children);
+    void simplify_children_of_mul(vector<nex_pow> & children, rational&);
 
 
     nex * clone(const nex* a) {        
@@ -113,6 +113,7 @@ public:
             for (const auto& p : m->children()) {
                 r->add_child_in_power(clone(p.e()), p.pow());
             }
+            r->coeff() = m->coeff();
             return r;
         }
         case expr_type::SUM: {
@@ -163,7 +164,7 @@ public:
     void add_children(T) { }
     
     template <typename T, typename K, typename ...Args>
-    void add_children(T r, K e, Args ...  es) {
+    void add_children(T r, K e, Args ...  es) {        
         r->add_child(e);
         add_children(r, es ...);
     }
@@ -247,11 +248,11 @@ public:
 
     void simplify_children_of_sum(ptr_vector<nex> & children);
     
-    bool eat_scalar_pow(nex_scalar *& r, nex_pow& p, unsigned);
+    bool eat_scalar_pow(rational& r, const nex_pow& p, unsigned);
     void simplify_children_of_mul(vector<nex_pow> & children, lt_on_vars lt, std::function<nex_scalar*()> mk_scalar);
 
     bool lt(const nex* a, const nex* b) const;    
-    bool less_than_on_mul(const nex_mul* a, const nex_mul* b) const;
+    bool less_than_on_mul_mul(const nex_mul* a, const nex_mul* b) const;
     bool less_than_on_var_nex(const nex_var* a, const nex* b) const;
     bool less_than_on_mul_nex(const nex_mul* a, const nex* b) const;
     bool less_than_on_sum_sum(const nex_sum* a, const nex_sum* b) const;

src/math/lp/nla_grobner.cpp

@@ -608,10 +608,6 @@ std::ostream& nla_grobner::display_equation(std::ostream & out, const equation &
     return out;
 }
 
-void nla_grobner::display_monomial(std::ostream & out, monomial const & m) const {
-    NOT_IMPLEMENTED_YET();
-}
-
 void nla_grobner::display(std::ostream & out) const {
     NOT_IMPLEMENTED_YET();
 }

src/math/lp/nla_grobner.h

@@ -38,22 +38,6 @@ struct grobner_stats {
 
 
 class nla_grobner : common {
-
-    struct monomial {
-        rational         m_coeff;
-        svector<lpvar>   m_vars;  //!< sorted variables
-    
-        friend class grobner;
-        friend struct monomial_lt;
-    
-        monomial() {}
-    public:
-        rational const & get_coeff() const { return m_coeff; }
-        unsigned get_degree() const { return m_vars.size(); }
-        unsigned get_size() const { return get_degree(); }
-        lpvar    get_var(unsigned idx) const { return m_vars[idx]; }
-    };
-
     class equation {
         unsigned                   m_bidx:31;       //!< position at m_equations_to_delete
         unsigned                   m_lc:1;          //!< true if equation if a linear combination of the input equations.
@@ -127,8 +111,6 @@ bool simplify_processed_with_eq(equation*);
     void display_equations(std::ostream & out, equation_set const & v, char const * header) const;
     std::ostream& display_equation(std::ostream & out, const equation & eq);
 
-    void display_monomial(std::ostream & out, monomial const & m) const;
-
     void display(std::ostream & out) const;
     void get_equations(ptr_vector<equation>& eqs);
     bool try_to_modify_eqs(ptr_vector<equation>& eqs, unsigned& next_weight);
@@ -138,13 +120,8 @@ bool simplify_processed_with_eq(equation*);
     void process_var(nex_mul*, lpvar j, ci_dependency *& dep, rational&);
 
     nex* mk_monomial_in_row(rational, lpvar, ci_dependency*&);
-    rational get_monomial_coeff(const nex_mul* m) {        
-        const nex* a = m->children()[0].e();
-        if (a->is_scalar())
-            return static_cast<const nex_scalar*>(a)->value();
-        return rational(1);
-    }
 
+    
     void init_equation(equation* eq, const nex*, ci_dependency* d);
     equation * simplify(equation const * source, equation * target);    
     // bool less_than_on_vars(lpvar a, lpvar b) const {

src/test/lp/lp.cpp

@@ -85,9 +85,9 @@ void test_simplify() {
         [](unsigned) { return false; },
         []() { return 1; }, // for random
                     r);
-    // enable_trace("nla_cn");
+    enable_trace("nla_cn");
-    // enable_trace("nla_cn_details");
+    enable_trace("nla_cn_details");
-    //    enable_trace("nla_cn_details_");
+    // enable_trace("nla_cn_details_");
     enable_trace("nla_test");
     
     r.set_number_of_vars(3);
@@ -98,6 +98,10 @@ void test_simplify() {
     nex_var* c = r.mk_var(2);
     auto bc = r.mk_mul(b, c);
     auto a_plus_bc = r.mk_sum(a, bc);
+    auto two_a_plus_bc = r.mk_mul(r.mk_scalar(rational(2)), a_plus_bc);
+    auto simp_two_a_plus_bc = r.simplify(two_a_plus_bc);
+    TRACE("nla_test", tout << * simp_two_a_plus_bc << "\n";);
+    SASSERT(nex_creator::equal(simp_two_a_plus_bc, two_a_plus_bc));
     auto simp_a_plus_bc = r.simplify(a_plus_bc);
     SASSERT(to_sum(simp_a_plus_bc)->size() > 1);
     auto three_ab = r.mk_mul(r.mk_scalar(rational(3)), a, b);
@@ -106,8 +110,8 @@ void test_simplify() {
     TRACE("nla_test", tout << "before simplify " << *three_ab_square << "\n";);
     three_ab_square = to_mul(r.simplify(three_ab_square));
     TRACE("nla_test", tout << *three_ab_square << "\n";);
-    nex_scalar * s = to_scalar(three_ab_square->children()[0].e());
+    const rational& s = three_ab_square->coeff();
-    SASSERT(s->value() == rational(27));
+    SASSERT(s == rational(27));
     auto m = r.mk_mul(); m->add_child_in_power(c, 2);
     TRACE("nla_test_", tout << "m = " << *m << "\n";); 
     auto n = r.mk_mul(a);
@@ -161,6 +165,7 @@ void test_cn_shorter() {
     enable_trace("nla_cn");
     enable_trace("nla_cn_test");
     enable_trace("nla_cn_details");
+    //    enable_trace("nla_cn_details_");
     enable_trace("nla_test_details");
     cr.set_number_of_vars(20);
     for (unsigned j = 0; j < cr.get_number_of_vars(); j++)