From 2a72d11f1407a3510e1f00c83744563cb357a57a Mon Sep 17 00:00:00 2001 From: Daniel Butter Date: Sun, 3 Aug 2025 21:17:56 -0400 Subject: [PATCH 1/8] Modifications to Compare.cc to test changes replacement_map format. --- core/Compare.cc | 330 +++++++++++++++++++++++++++++++++- core/Compare.hh | 26 +++ core/Storage.hh | 3 +- core/algorithms/substitute.cc | 12 +- 4 files changed, 360 insertions(+), 11 deletions(-) diff --git a/core/Compare.cc b/core/Compare.cc index 41fddff3cd..97043b74e1 100644 --- a/core/Compare.cc +++ b/core/Compare.cc @@ -24,7 +24,7 @@ namespace cadabra { int Ex_comparator::offset=0; - + int subtree_compare(const Properties *properties, Ex::iterator one, Ex::iterator two, int mod_prel, bool, int compare_multiplier, bool literal_wildcards) @@ -137,6 +137,282 @@ namespace cadabra { return remember_ret; } + int subtree_compare(const Properties *properties, + Lazy_Ex one_lazy, Lazy_Ex two_lazy, + int mod_prel, bool, int compare_multiplier, bool literal_wildcards) + { + Ex::iterator one = one_lazy.first; + Ex::iterator two = two_lazy.first; + + // std::cerr << "comparing " << Ex(one) << " with " << Ex(two) << " " << mod_prel << ", " << checksets << std::endl; + + // The logic is to compare successive aspects of the two objects, returning a + // no-match code if a difference is found at a particular level, or continuing + // further down the line if there still is a match. + + // Compare multipliers. Skip this step if one of the objects is a rational and the + // other not, as in that case we are matching values to symbols. + if( one->is_rational()==two->is_rational() ) { + if(compare_multiplier==-2 && !two->is_name_wildcard() && !one->is_name_wildcard()) + if(one->multiplier != two->multiplier) { + if(*one->multiplier < *two->multiplier) return 2; + else return -2; + } + } + + int mult=2; + // Handle object wildcards and comparison + if(!literal_wildcards) { + if(one->is_object_wildcard() || two->is_object_wildcard()) + return 0; + } + + // Handle mismatching node names. + if(one->name!=two->name) { + // std::cerr << *one->name << " != " << *two->name << std::endl; + if(literal_wildcards) { + if(*one->name < *two->name) return mult; + else return -mult; + } + + DEBUGLN( std::cerr << "auto? " << one->is_autodeclare_wildcard() << ", " << two->is_numbered_symbol() << std::endl; ); + if( (one->is_autodeclare_wildcard() && two->is_numbered_symbol()) || + (two->is_autodeclare_wildcard() && one->is_numbered_symbol()) ) { + if( one->name_only() != two->name_only() ) { + if(*one->name < *two->name) return mult; + else return -mult; + } + } + else if( one->is_name_wildcard()==false && two->is_name_wildcard()==false ) { + if(*one->name < *two->name) return mult; + else return -mult; + } + + DEBUGLN( std::cerr << "match despite difference: " << *one->name << " and " << *two->name << std::endl; ); + } + + // Compare parent relations. + if(mod_prel<=-2) { + str_node::parent_rel_t p1=one->fl.parent_rel; + str_node::parent_rel_t p2=two->fl.parent_rel; + + if (one_lazy.second == repl_t::erase_parent_rel) { + p1 = str_node::parent_rel_t::p_none; + } else if (one_lazy.second == repl_t::flip_parent_rel) { + if(p1==str_node::parent_rel_t::p_super) p1=str_node::parent_rel_t::p_sub; + else if(p1==str_node::parent_rel_t::p_sub) p1=str_node::parent_rel_t::p_super; + else throw std::logic_error("flip_parent_rel called on non-index"); + } + + if (two_lazy.second == repl_t::erase_parent_rel) { + p2 = str_node::parent_rel_t::p_none; + } else if (two_lazy.second == repl_t::flip_parent_rel) { + if(p2==str_node::parent_rel_t::p_super) p2=str_node::parent_rel_t::p_sub; + else if(p2==str_node::parent_rel_t::p_sub) p2=str_node::parent_rel_t::p_super; + else throw std::logic_error("flip_parent_rel called on non-index"); + } + + if(p1!=p2) { + return (p10) --mod_prel; + if(compare_multiplier==0) compare_multiplier=-2; + else if(compare_multiplier>0) --compare_multiplier; + + for(;;) { + Ex::sibling_iterator sib1=one.begin(), sib2=two.begin(); + + while(sib1!=one.end()) { + if(sib1->is_index() == do_indices) { + int ret=subtree_compare(properties, sib1,sib2, mod_prel, true /* checksets */, + compare_multiplier, literal_wildcards); + // std::cerr << "result " << ret << std::endl; + if(abs(ret)>1) + return ret/abs(ret)*mult; + if(ret!=0 && remember_ret==0) + remember_ret=ret; + } + ++sib1; + ++sib2; + } + + if(remember_ret!=0) break; + + if(!do_indices) do_indices=true; + else break; + } + + return remember_ret; + } + + int subtree_compare(const Properties *properties, + Lazy_Ex one_lazy, Lazy_Ex two_lazy, + int mod_prel, bool, int compare_multiplier, bool literal_wildcards) + { + Ex::iterator one = one_lazy.first; + Ex::iterator two = two_lazy.first; + + // std::cerr << "comparing " << Ex(one) << " with " << Ex(two) << " " << mod_prel << ", " << checksets << std::endl; + + // The logic is to compare successive aspects of the two objects, returning a + // no-match code if a difference is found at a particular level, or continuing + // further down the line if there still is a match. + + // Compare multipliers. Skip this step if one of the objects is a rational and the + // other not, as in that case we are matching values to symbols. + if( one->is_rational()==two->is_rational() ) { + if(compare_multiplier==-2 && !two->is_name_wildcard() && !one->is_name_wildcard()) + if(one->multiplier != two->multiplier) { + if(*one->multiplier < *two->multiplier) return 2; + else return -2; + } + } + + int mult=2; + // Handle object wildcards and comparison + if(!literal_wildcards) { + if(one->is_object_wildcard() || two->is_object_wildcard()) + return 0; + } + + // Handle mismatching node names. + if(one->name!=two->name) { + // std::cerr << *one->name << " != " << *two->name << std::endl; + if(literal_wildcards) { + if(*one->name < *two->name) return mult; + else return -mult; + } + + DEBUGLN( std::cerr << "auto? " << one->is_autodeclare_wildcard() << ", " << two->is_numbered_symbol() << std::endl; ); + if( (one->is_autodeclare_wildcard() && two->is_numbered_symbol()) || + (two->is_autodeclare_wildcard() && one->is_numbered_symbol()) ) { + if( one->name_only() != two->name_only() ) { + if(*one->name < *two->name) return mult; + else return -mult; + } + } + else if( one->is_name_wildcard()==false && two->is_name_wildcard()==false ) { + if(*one->name < *two->name) return mult; + else return -mult; + } + + DEBUGLN( std::cerr << "match despite difference: " << *one->name << " and " << *two->name << std::endl; ); + } + + // Compare parent relations. + if(mod_prel<=-2) { + str_node::parent_rel_t p1=one->fl.parent_rel; + str_node::parent_rel_t p2=two->fl.parent_rel; + + if (one_lazy.second == repl_t::erase_parent_rel) { + p1 = str_node::parent_rel_t::p_none; + } else if (one_lazy.second == repl_t::flip_parent_rel) { + if(p1==str_node::parent_rel_t::p_super) p1=str_node::parent_rel_t::p_sub; + else if(p1==str_node::parent_rel_t::p_sub) p1=str_node::parent_rel_t::p_super; + else throw std::logic_error("flip_parent_rel called on non-index"); + } + + if (two_lazy.second == repl_t::erase_parent_rel) { + p2 = str_node::parent_rel_t::p_none; + } else if (two_lazy.second == repl_t::flip_parent_rel) { + if(p2==str_node::parent_rel_t::p_super) p2=str_node::parent_rel_t::p_sub; + else if(p2==str_node::parent_rel_t::p_sub) p2=str_node::parent_rel_t::p_super; + else throw std::logic_error("flip_parent_rel called on non-index"); + } + + if(p1!=p2) { + return (p10) --mod_prel; + if(compare_multiplier==0) compare_multiplier=-2; + else if(compare_multiplier>0) --compare_multiplier; + + for(;;) { + Ex::sibling_iterator sib1=one.begin(), sib2=two.begin(); + + while(sib1!=one.end()) { + if(sib1->is_index() == do_indices) { + int ret=subtree_compare(properties, sib1,sib2, mod_prel, true /* checksets */, + compare_multiplier, literal_wildcards); + // std::cerr << "result " << ret << std::endl; + if(abs(ret)>1) + return ret/abs(ret)*mult; + if(ret!=0 && remember_ret==0) + remember_ret=ret; + } + ++sib1; + ++sib2; + } + + if(remember_ret!=0) break; + + if(!do_indices) do_indices=true; + else break; + } + + return remember_ret; + } + bool tree_less(const Properties* properties, const Ex& one, const Ex& two, int mod_prel, bool checksets, int compare_multiplier) { return subtree_less(properties, one.begin(), two.begin(), mod_prel, checksets, compare_multiplier); @@ -286,6 +562,7 @@ namespace cadabra { void Ex_comparator::clear() { replacement_map.clear(); + new_replacement_map.clear(); subtree_replacement_map.clear(); index_value_map.clear(); factor_locations.clear(); @@ -597,6 +874,7 @@ namespace cadabra { // below) so that searching for rules can remain simple. replacement_map_t::iterator loc=replacement_map.find(Ex(one)); + new_replacement_map_t::iterator new_loc=new_replacement_map.find({one, repl_t::same}); bool tested_full=true; @@ -604,17 +882,20 @@ namespace cadabra { // also search the pattern without the children (but not if // this node is an Accent, because then the child nodes are // very relevant). - if(loc == replacement_map.end() && Ex::number_of_children(one)!=0) { + assert( (new_loc == new_replacement_map.end()) == (loc == replacement_map.end()) ); + if(new_loc == new_replacement_map.end() && Ex::number_of_children(one)!=0) { DEBUGLN( std::cerr << tab() << "**** not found, trying without child nodes" << std::endl; ); if(properties.get(one)==0 ) { Ex tmp1(one); tmp1.erase_children(tmp1.begin()); loc = replacement_map.find(tmp1); + new_loc = new_replacement_map.find({one, repl_t::erase_children}); tested_full=false; } } - if(loc!=replacement_map.end()) { + assert( (new_loc == new_replacement_map.end()) == (loc == replacement_map.end()) ); + if(new_loc != new_replacement_map.end()) { // We constructed a replacement rule for this node already at an earlier // stage. Need to make sure that that rule is consistent with what we // found now. @@ -622,12 +903,16 @@ namespace cadabra { int cmp; // If this is an index/pattern, try to match the whole index/pattern. - if(tested_full) - cmp=subtree_compare(&properties, (*loc).second.begin(), two, -2); + if(tested_full) { + cmp = subtree_compare(&properties, new_loc->first, {two, repl_t::same}, -2); + assert(cmp == subtree_compare(&properties, (*loc).second.begin(), two, -2)); + } else { Ex tmp2(two); tmp2.erase_children(tmp2.begin()); - cmp=subtree_compare(&properties, (*loc).second.begin(), tmp2.begin(), -2); + // FIXME(Dan): Why not just do subtree_compare *without* children? + cmp = subtree_compare(&properties, new_loc->first, {two, repl_t::erase_children}, -2); + assert(cmp==subtree_compare(&properties, (*loc).second.begin(), tmp2.begin(), -2)); } DEBUGLN(std::cerr << " pattern " << two << " should be " << (*loc).second.begin() @@ -747,15 +1032,18 @@ namespace cadabra { Ex tmp(two); // cadabra::one(tmp.begin()->multiplier); replacement_map[Ex(one)]=tmp; - // if this is an index, also store the pattern with the parent_rel flipped if(one->is_index()) { Ex cmptree1(one); Ex cmptree2(two); cmptree1.begin()->flip_parent_rel(); - if(two->is_index()) + if(two->is_index()) { cmptree2.begin()->flip_parent_rel(); + new_replacement_map[{one, repl_t::flip_parent_rel}] = {two, repl_t::flip_parent_rel}; + } else { + new_replacement_map[{one, repl_t::flip_parent_rel}] = {two, repl_t::same}; + } replacement_map[cmptree1]=cmptree2; } @@ -766,6 +1054,7 @@ namespace cadabra { tmp1.erase_children(tmp1.begin()); tmp2.erase_children(tmp2.begin()); replacement_map[tmp1]=tmp2; + new_replacement_map[{one, repl_t::erase_children}]={two, repl_t::erase_children}; } // and if this is a pattern also insert the one without the parent_rel if( /* one->is_name_wildcard() && */ one->is_index()) { @@ -775,12 +1064,14 @@ namespace cadabra { tmp2.begin()->fl.parent_rel=str_node::p_none; // We do not want this pattern to be present already. auto fnd=replacement_map.find(tmp1); + auto new_fnd=new_replacement_map.find({one, repl_t::erase_parent_rel}); if(fnd!=replacement_map.end()) { throw InternalError("Attempting to duplicate replacement rule."); } // assert(replacement_map.find(tmp1)!=replacement_map.end()); // std::cerr << "storing " << Ex(tmp1) << " -> " << Ex(tmp2) << std::endl; replacement_map[tmp1]=tmp2; + new_replacement_map[{one, repl_t::erase_parent_rel}]={two, repl_t::erase_parent_rel}; } DEBUGLN( std::cerr << "Replacement map is now:" << std::endl; @@ -959,6 +1250,7 @@ namespace cadabra { Ex::iterator conditions) { replacement_map_t backup_replacements(replacement_map); + new_replacement_map_t new_backup_replacements(new_replacement_map); subtree_replacement_map_t backup_subtree_replacements(subtree_replacement_map); // 'Start' iterates over all factors, trying to find 'tofind'. It may happen that the @@ -1000,6 +1292,8 @@ namespace cadabra { } if(sign==0) { // object found, but we cannot move it in the right order replacement_map=backup_replacements; + new_replacement_map=new_backup_replacements; + // replacement_map.revert(); subtree_replacement_map=backup_subtree_replacements; } else { @@ -1010,12 +1304,15 @@ namespace cadabra { ++nxt; if(nxt!=lhs.end()) { match_t res=match_subproduct(tr, lhs, nxt, st, conditions); + if(res==match_t::subtree_match) return res; else { // txtout << tofind.node << "found factor useless " << start.node << std::endl; factor_locations.pop_back(); factor_moving_signs.pop_back(); replacement_map=backup_replacements; + new_replacement_map=new_backup_replacements; + // replacement_map.revert(); subtree_replacement_map=backup_subtree_replacements; } } @@ -1030,6 +1327,8 @@ namespace cadabra { factor_locations.pop_back(); factor_moving_signs.pop_back(); replacement_map=backup_replacements; + new_replacement_map=new_backup_replacements; + // replacement_map.revert(); subtree_replacement_map=backup_subtree_replacements; } } @@ -1038,6 +1337,8 @@ namespace cadabra { else { // txtout << tofind.node << "does not match" << std::endl; replacement_map=backup_replacements; + new_replacement_map=new_backup_replacements; + // replacement_map.revert(); subtree_replacement_map=backup_subtree_replacements; } } @@ -1053,6 +1354,7 @@ namespace cadabra { Ex::iterator conditions) { replacement_map_t backup_replacements(replacement_map); + // replacement_map.checkpoint(); subtree_replacement_map_t backup_subtree_replacements(subtree_replacement_map); // 'Start' iterates over all terms, trying to find 'tofind'. It may happen that the @@ -1083,6 +1385,7 @@ namespace cadabra { auto this_ratio = (*tofind->multiplier)/(*start->multiplier); if(this_ratio!=term_ratio) { replacement_map=backup_replacements; + // replacement_map.revert(); subtree_replacement_map=backup_subtree_replacements; } else { @@ -1096,6 +1399,7 @@ namespace cadabra { else { factor_locations.pop_back(); replacement_map=backup_replacements; + // replacement_map.revert(); subtree_replacement_map=backup_subtree_replacements; } } @@ -1109,6 +1413,7 @@ namespace cadabra { else { factor_locations.pop_back(); replacement_map=backup_replacements; + // replacement_map.revert(); subtree_replacement_map=backup_subtree_replacements; } } @@ -1117,6 +1422,7 @@ namespace cadabra { else { // txtout << tofind.node << "does not match" << std::endl; replacement_map=backup_replacements; + // replacement_map.revert(); subtree_replacement_map=backup_subtree_replacements; } } @@ -1621,6 +1927,7 @@ namespace cadabra { if(replacement_map.find(Ex(lhs))==replacement_map.end() || replacement_map.find(Ex(rhs))==replacement_map.end()) return true; // std::cerr << *lhs->name << " !=?? " << *rhs->name << std::endl; + // FIXME(Dan): Above and Below need to be modified if(tree_exact_equal(&properties, replacement_map[Ex(lhs)], replacement_map[Ex(rhs)])) { return false; } @@ -1703,6 +2010,7 @@ namespace cadabra { // txtout << "matching " << *comp.replacement_map[lhs->name] // << " with pattern " << pat << std::endl; std::regex reg(pat); + // FIXME(Dan): Below if (!std::regex_match(*(replacement_map[Ex(lhs)].begin()->name), reg)) return false; } @@ -1773,6 +2081,12 @@ namespace cadabra { else return false; } + + bool lazy_less::operator()(const Lazy_Ex& one, const Lazy_Ex& two) const { + return subtree_compare(nullptr, one, two, -2, true, 0, true) > 0; + } + + } bool operator<(const cadabra::Ex::iterator& i1, const cadabra::Ex::iterator& i2) diff --git a/core/Compare.hh b/core/Compare.hh index 7845453d0e..68cb4ee194 100644 --- a/core/Compare.hh +++ b/core/Compare.hh @@ -58,6 +58,28 @@ namespace cadabra { int mod_prel=-2, bool checksets=true, int compare_multiplier=-2, bool literal_wildcards=false); + + + // Introduce a lazy replacement type to avoid temporary Ex objects + enum class repl_t {same, // identity operation + flip_parent_rel, // flip top node's parent_rel + erase_parent_rel, // erase top node's parent_rel + erase_children // ignore any children + }; + typedef std::pair Lazy_Ex; + class lazy_less { + public: + bool operator()(const Lazy_Ex& first, const Lazy_Ex& second) const; + + }; + + int subtree_compare(const Properties*, + Lazy_Ex one, Lazy_Ex two, + int mod_prel=-2, bool checksets=true, int compare_multiplier=-2, + bool literal_wildcards=false); + + + /// Various comparison functions, some exact, some with pattern logic. bool tree_less(const Properties*, @@ -277,7 +299,11 @@ namespace cadabra { /// Map for the replacement of nodes (indices, patterns). typedef std::map replacement_map_t; + typedef std::map new_replacement_map_t; + + // typedef checkpoint_map replacement_map_t; replacement_map_t replacement_map; + new_replacement_map_t new_replacement_map; /// Map for the replacement of entire subtrees (object patterns). diff --git a/core/Storage.hh b/core/Storage.hh index 1ecfba2507..0061c1eb99 100644 --- a/core/Storage.hh +++ b/core/Storage.hh @@ -36,11 +36,12 @@ You should have received a copy of the GNU General Public License #include "tree.hh" #include "Multiplier.hh" + namespace cadabra { class NTensor; class NInterpolatingFunction; - + typedef Multiplier multiplier_t; typedef std::set nset_t; typedef std::set rset_t; diff --git a/core/algorithms/substitute.cc b/core/algorithms/substitute.cc index 1ec422e70c..2c8ec3d5e5 100644 --- a/core/algorithms/substitute.cc +++ b/core/algorithms/substitute.cc @@ -155,7 +155,7 @@ bool substitute::can_apply(iterator st) ret == Ex_comparator::match_t::match_index_less || ret == Ex_comparator::match_t::match_index_greater) { use_rule=arrow; - + // std::cout << comparator.replacement_map.print_tree() << "\n"; // If we are not matching a partial sum or partial product, need to check that all // terms or factors are accounted for. if(!partial) { @@ -235,6 +235,10 @@ Algorithm::result_t substitute::apply(iterator& st) iterator it=repl.begin(); Ex_comparator::replacement_map_t::iterator loc; Ex_comparator::subtree_replacement_map_t::iterator sloc; + + + Ex_comparator::new_replacement_map_t::iterator nloc; + std::vector subtree_insertion_points; while(it!=repl.end()) { bool is_stripped=false; @@ -243,15 +247,18 @@ Algorithm::result_t substitute::apply(iterator& st) // match ^{a?}. (though this does match when we write 'i' instead of a?. loc=comparator.replacement_map.find(Ex(it)); + nloc = comparator.new_replacement_map.find({it, repl_t::same}); + assert( (loc==comparator.replacement_map.end()) == (nloc==comparator.new_replacement_map.end()) ); if(loc==comparator.replacement_map.end() && it->is_name_wildcard() && tr.number_of_children(it)!=0) { Ex tmp(it); tmp.erase_children(tmp.begin()); loc=comparator.replacement_map.find(tmp); + nloc=comparator.new_replacement_map.find({it, repl_t::erase_children}); is_stripped=true; } //std::cerr << "consider element of repl " << Ex(it) << std::endl; - + assert( (loc==comparator.replacement_map.end()) == (nloc==comparator.new_replacement_map.end()) ); if(loc!=comparator.replacement_map.end()) { // name wildcards DEBUGLN( std::cerr << "wildcard replaced: " << loc->first << " -> " << loc->second << std::endl; ) @@ -274,6 +281,7 @@ Algorithm::result_t substitute::apply(iterator& st) it->name=(*loc).second.begin()->name; // See the comment below about multipliers. multiply(it->multiplier, *(*loc).second.begin()->multiplier); + // assert((*loc).second.begin()->fl == (*nloc).second.first.begin()->fl); it->fl=(*loc).second.begin()->fl; } else { From eabc01f68fd25983a088ddaf5043b5499a2135a0 Mon Sep 17 00:00:00 2001 From: Daniel Butter Date: Mon, 4 Aug 2025 10:42:43 -0400 Subject: [PATCH 2/8] Finished adding new replacement_map code to Compare.cc. Modified substitute to use this. --- core/Compare.cc | 209 +++++++++++++++++++++++----------- core/Compare.hh | 2 + core/algorithms/substitute.cc | 60 +++++++--- 3 files changed, 187 insertions(+), 84 deletions(-) diff --git a/core/Compare.cc b/core/Compare.cc index 97043b74e1..a85b453c4d 100644 --- a/core/Compare.cc +++ b/core/Compare.cc @@ -1,3 +1,17 @@ +#ifdef NDEBUG +# undef NDEBUG +#endif +#include + +// #define TEST + +#ifdef TEST + #define IF_TEST(...) __VA_ARGS__ +#else + #define IF_TEST(...) +#endif + + #include @@ -138,11 +152,11 @@ namespace cadabra { } int subtree_compare(const Properties *properties, - Lazy_Ex one_lazy, Lazy_Ex two_lazy, + Lazy_Ex lazy_one, Lazy_Ex lazy_two, int mod_prel, bool, int compare_multiplier, bool literal_wildcards) { - Ex::iterator one = one_lazy.first; - Ex::iterator two = two_lazy.first; + Ex::iterator one = lazy_one.first; + Ex::iterator two = lazy_two.first; // std::cerr << "comparing " << Ex(one) << " with " << Ex(two) << " " << mod_prel << ", " << checksets << std::endl; @@ -196,17 +210,17 @@ namespace cadabra { str_node::parent_rel_t p1=one->fl.parent_rel; str_node::parent_rel_t p2=two->fl.parent_rel; - if (one_lazy.second == repl_t::erase_parent_rel) { + if (lazy_one.second == repl_t::erase_parent_rel) { p1 = str_node::parent_rel_t::p_none; - } else if (one_lazy.second == repl_t::flip_parent_rel) { + } else if (lazy_one.second == repl_t::flip_parent_rel) { if(p1==str_node::parent_rel_t::p_super) p1=str_node::parent_rel_t::p_sub; else if(p1==str_node::parent_rel_t::p_sub) p1=str_node::parent_rel_t::p_super; else throw std::logic_error("flip_parent_rel called on non-index"); } - if (two_lazy.second == repl_t::erase_parent_rel) { + if (lazy_two.second == repl_t::erase_parent_rel) { p2 = str_node::parent_rel_t::p_none; - } else if (two_lazy.second == repl_t::flip_parent_rel) { + } else if (lazy_two.second == repl_t::flip_parent_rel) { if(p2==str_node::parent_rel_t::p_super) p2=str_node::parent_rel_t::p_sub; else if(p2==str_node::parent_rel_t::p_sub) p2=str_node::parent_rel_t::p_super; else throw std::logic_error("flip_parent_rel called on non-index"); @@ -219,8 +233,8 @@ namespace cadabra { // Now turn to the child nodes. Before comparing them directly, first compare // the number of children, taking into account range wildcards. - int numch1 = (one_lazy.second == repl_t::erase_children) ? 0 : Ex::number_of_children(one); - int numch2 = (two_lazy.second == repl_t::erase_children) ? 0 : Ex::number_of_children(two); + int numch1 = (lazy_one.second == repl_t::erase_children) ? 0 : Ex::number_of_children(one); + int numch2 = (lazy_two.second == repl_t::erase_children) ? 0 : Ex::number_of_children(two); // FIXME: handle range wildcards as in Props.cc @@ -229,7 +243,7 @@ namespace cadabra { else return -2; } - // assert(numch1 == numch2) + assert(numch1 == numch2); // Return equal if both have no children if (numch1 == 0) { return 0; @@ -257,6 +271,7 @@ namespace cadabra { int ret=subtree_compare(properties, sib1,sib2, mod_prel, true /* checksets */, compare_multiplier, literal_wildcards); // std::cerr << "result " << ret << std::endl; + // FIXME(Dan): The below could be modified? if(abs(ret)>1) return ret/abs(ret)*mult; if(ret!=0 && remember_ret==0) @@ -276,11 +291,11 @@ namespace cadabra { } int subtree_compare(const Properties *properties, - Lazy_Ex one_lazy, Lazy_Ex two_lazy, + Lazy_Ex lazy_one, Lazy_Ex lazy_two, int mod_prel, bool, int compare_multiplier, bool literal_wildcards) { - Ex::iterator one = one_lazy.first; - Ex::iterator two = two_lazy.first; + Ex::iterator one = lazy_one.first; + Ex::iterator two = lazy_two.first; // std::cerr << "comparing " << Ex(one) << " with " << Ex(two) << " " << mod_prel << ", " << checksets << std::endl; @@ -334,17 +349,17 @@ namespace cadabra { str_node::parent_rel_t p1=one->fl.parent_rel; str_node::parent_rel_t p2=two->fl.parent_rel; - if (one_lazy.second == repl_t::erase_parent_rel) { + if (lazy_one.second == repl_t::erase_parent_rel) { p1 = str_node::parent_rel_t::p_none; - } else if (one_lazy.second == repl_t::flip_parent_rel) { + } else if (lazy_one.second == repl_t::flip_parent_rel) { if(p1==str_node::parent_rel_t::p_super) p1=str_node::parent_rel_t::p_sub; else if(p1==str_node::parent_rel_t::p_sub) p1=str_node::parent_rel_t::p_super; else throw std::logic_error("flip_parent_rel called on non-index"); } - if (two_lazy.second == repl_t::erase_parent_rel) { + if (lazy_two.second == repl_t::erase_parent_rel) { p2 = str_node::parent_rel_t::p_none; - } else if (two_lazy.second == repl_t::flip_parent_rel) { + } else if (lazy_two.second == repl_t::flip_parent_rel) { if(p2==str_node::parent_rel_t::p_super) p2=str_node::parent_rel_t::p_sub; else if(p2==str_node::parent_rel_t::p_sub) p2=str_node::parent_rel_t::p_super; else throw std::logic_error("flip_parent_rel called on non-index"); @@ -357,8 +372,8 @@ namespace cadabra { // Now turn to the child nodes. Before comparing them directly, first compare // the number of children, taking into account range wildcards. - int numch1 = (one_lazy.second == repl_t::erase_children) ? 0 : Ex::number_of_children(one); - int numch2 = (two_lazy.second == repl_t::erase_children) ? 0 : Ex::number_of_children(two); + int numch1 = (lazy_one.second == repl_t::erase_children) ? 0 : Ex::number_of_children(one); + int numch2 = (lazy_two.second == repl_t::erase_children) ? 0 : Ex::number_of_children(two); // FIXME: handle range wildcards as in Props.cc @@ -367,7 +382,7 @@ namespace cadabra { else return -2; } - // assert(numch1 == numch2) + assert(numch1 == numch2); // Return equal if both have no children if (numch1 == 0) { return 0; @@ -395,6 +410,7 @@ namespace cadabra { int ret=subtree_compare(properties, sib1,sib2, mod_prel, true /* checksets */, compare_multiplier, literal_wildcards); // std::cerr << "result " << ret << std::endl; + // FIXME(Dan): The below could be modified? if(abs(ret)>1) return ret/abs(ret)*mult; if(ret!=0 && remember_ret==0) @@ -561,7 +577,7 @@ namespace cadabra { void Ex_comparator::clear() { - replacement_map.clear(); + IF_TEST(replacement_map.clear();); new_replacement_map.clear(); subtree_replacement_map.clear(); index_value_map.clear(); @@ -873,7 +889,9 @@ namespace cadabra { // triggering a rule for an upper index, we simply store both rules (see // below) so that searching for rules can remain simple. - replacement_map_t::iterator loc=replacement_map.find(Ex(one)); + IF_TEST(replacement_map_t::iterator loc=replacement_map.find(Ex(one));); + // FIXME(Dan): Modified to work directly with iterator + new_replacement_map_t::iterator new_loc=new_replacement_map.find({one, repl_t::same}); bool tested_full=true; @@ -882,19 +900,21 @@ namespace cadabra { // also search the pattern without the children (but not if // this node is an Accent, because then the child nodes are // very relevant). - assert( (new_loc == new_replacement_map.end()) == (loc == replacement_map.end()) ); + IF_TEST(assert( (new_loc == new_replacement_map.end()) == (loc == replacement_map.end()) ); ); if(new_loc == new_replacement_map.end() && Ex::number_of_children(one)!=0) { DEBUGLN( std::cerr << tab() << "**** not found, trying without child nodes" << std::endl; ); if(properties.get(one)==0 ) { - Ex tmp1(one); - tmp1.erase_children(tmp1.begin()); - loc = replacement_map.find(tmp1); + IF_TEST ( + Ex tmp1(one); + tmp1.erase_children(tmp1.begin()); + loc = replacement_map.find(tmp1); + ); new_loc = new_replacement_map.find({one, repl_t::erase_children}); tested_full=false; } } - assert( (new_loc == new_replacement_map.end()) == (loc == replacement_map.end()) ); + IF_TEST( assert( (new_loc == new_replacement_map.end()) == (loc == replacement_map.end()) ); ); if(new_loc != new_replacement_map.end()) { // We constructed a replacement rule for this node already at an earlier // stage. Need to make sure that that rule is consistent with what we @@ -904,15 +924,16 @@ namespace cadabra { // If this is an index/pattern, try to match the whole index/pattern. if(tested_full) { - cmp = subtree_compare(&properties, new_loc->first, {two, repl_t::same}, -2); - assert(cmp == subtree_compare(&properties, (*loc).second.begin(), two, -2)); + cmp = subtree_compare(&properties, new_loc->second, {two, repl_t::same}, -2); + IF_TEST(assert(cmp == subtree_compare(&properties, (*loc).second.begin(), two, -2));); } else { - Ex tmp2(two); - tmp2.erase_children(tmp2.begin()); - // FIXME(Dan): Why not just do subtree_compare *without* children? - cmp = subtree_compare(&properties, new_loc->first, {two, repl_t::erase_children}, -2); - assert(cmp==subtree_compare(&properties, (*loc).second.begin(), tmp2.begin(), -2)); + cmp = subtree_compare(&properties, new_loc->second, {two, repl_t::erase_children}, -2); + IF_TEST ( + Ex tmp2(two); + tmp2.erase_children(tmp2.begin()); + assert(cmp==subtree_compare(&properties, (*loc).second.begin(), tmp2.begin(), -2)); + ); } DEBUGLN(std::cerr << " pattern " << two << " should be " << (*loc).second.begin() @@ -1029,48 +1050,62 @@ namespace cadabra { // We absolutely need to keep the multiplier here. If we don't, then it // becomes very messy to restore multipliers of child nodes in the pattern, // e.g A?_{m n} matching Q_{1 -1}. Do not set the multiplier to one! - Ex tmp(two); - // cadabra::one(tmp.begin()->multiplier); - replacement_map[Ex(one)]=tmp; + IF_TEST( + Ex tmp(two); + // cadabra::one(tmp.begin()->multiplier); + replacement_map[Ex(one)]=tmp; + ); + new_replacement_map[{one, repl_t::same}] = {two, repl_t::same}; + // if this is an index, also store the pattern with the parent_rel flipped + IF_TEST( + if(one->is_index()) { + Ex cmptree1(one); + Ex cmptree2(two); + cmptree1.begin()->flip_parent_rel(); + if(two->is_index()) + cmptree2.begin()->flip_parent_rel(); + replacement_map[cmptree1]=cmptree2; + } + ); + if(one->is_index()) { - Ex cmptree1(one); - Ex cmptree2(two); - cmptree1.begin()->flip_parent_rel(); - if(two->is_index()) { - cmptree2.begin()->flip_parent_rel(); + if(two->is_index()) new_replacement_map[{one, repl_t::flip_parent_rel}] = {two, repl_t::flip_parent_rel}; - } else { + else new_replacement_map[{one, repl_t::flip_parent_rel}] = {two, repl_t::same}; - } - replacement_map[cmptree1]=cmptree2; - } + } // if this is a pattern and the pattern has a non-zero number of children, // also add the pattern without the children if(Ex::number_of_children(one)!=0) { - Ex tmp1(one), tmp2(two); - tmp1.erase_children(tmp1.begin()); - tmp2.erase_children(tmp2.begin()); - replacement_map[tmp1]=tmp2; + IF_TEST( + Ex tmp1(one), tmp2(two); + tmp1.erase_children(tmp1.begin()); + tmp2.erase_children(tmp2.begin()); + replacement_map[tmp1]=tmp2; + ); new_replacement_map[{one, repl_t::erase_children}]={two, repl_t::erase_children}; } // and if this is a pattern also insert the one without the parent_rel if( /* one->is_name_wildcard() && */ one->is_index()) { //std::cerr << "storing pattern without pattern rel " << replacement_map.size() << std::endl; - Ex tmp1(one), tmp2(two); - tmp1.begin()->fl.parent_rel=str_node::p_none; - tmp2.begin()->fl.parent_rel=str_node::p_none; - // We do not want this pattern to be present already. - auto fnd=replacement_map.find(tmp1); auto new_fnd=new_replacement_map.find({one, repl_t::erase_parent_rel}); - if(fnd!=replacement_map.end()) { + IF_TEST( + Ex tmp1(one), tmp2(two); + tmp1.begin()->fl.parent_rel=str_node::p_none; + tmp2.begin()->fl.parent_rel=str_node::p_none; + // We do not want this pattern to be present already. + auto fnd=replacement_map.find(tmp1); + assert( (fnd==replacement_map.end()) == (new_fnd==new_replacement_map.end())); + ); + if(new_fnd!=new_replacement_map.end()) { throw InternalError("Attempting to duplicate replacement rule."); } // assert(replacement_map.find(tmp1)!=replacement_map.end()); // std::cerr << "storing " << Ex(tmp1) << " -> " << Ex(tmp2) << std::endl; - replacement_map[tmp1]=tmp2; + IF_TEST( replacement_map[tmp1]=tmp2; ); new_replacement_map[{one, repl_t::erase_parent_rel}]={two, repl_t::erase_parent_rel}; } @@ -1249,7 +1284,12 @@ namespace cadabra { Ex::sibling_iterator st, Ex::iterator conditions) { - replacement_map_t backup_replacements(replacement_map); + bool swflag = !Stopwatches["internal"].stopped() & Stopwatches["can_move_adjacent"].stopped(); + if (swflag) Stopwatches["match_subproduct"].start(); + if (swflag) Stopwatches["map_copies"].start(); + + IF_TEST(replacement_map_t backup_replacements(replacement_map);); + new_replacement_map_t new_backup_replacements(new_replacement_map); subtree_replacement_map_t backup_subtree_replacements(subtree_replacement_map); @@ -1258,6 +1298,13 @@ namespace cadabra { // to iterate 'start' over more factors (typically when non-commutative objects are // concerned). + // If the pattern is a product of m factors and we are looking at a product of k factors + // this takes O(k*m) in general. This compares the first factor in the pattern to every + // factor in the product until finding a match. Then it compares the second factor in the + // pattern to every factor in the product, until finding a match. + + // FIXME(Dan): Shortcut: if pattern product is too long, then we can skip. + Ex::sibling_iterator start=st.begin(); while(start!=st.end()) { @@ -1291,7 +1338,8 @@ namespace cadabra { DEBUGLN(std::cerr << "--- done can move ---" << sign << std::endl; ); } if(sign==0) { // object found, but we cannot move it in the right order - replacement_map=backup_replacements; + if (swflag) Stopwatches["map_copies"].start(); + IF_TEST(replacement_map=backup_replacements;); new_replacement_map=new_backup_replacements; // replacement_map.revert(); subtree_replacement_map=backup_subtree_replacements; @@ -1310,7 +1358,8 @@ namespace cadabra { // txtout << tofind.node << "found factor useless " << start.node << std::endl; factor_locations.pop_back(); factor_moving_signs.pop_back(); - replacement_map=backup_replacements; + if (swflag) Stopwatches["map_copies"].start(); + IF_TEST(replacement_map=backup_replacements;); new_replacement_map=new_backup_replacements; // replacement_map.revert(); subtree_replacement_map=backup_subtree_replacements; @@ -1326,7 +1375,8 @@ namespace cadabra { else { factor_locations.pop_back(); factor_moving_signs.pop_back(); - replacement_map=backup_replacements; + if (swflag) Stopwatches["map_copies"].start(); + IF_TEST(replacement_map=backup_replacements;); new_replacement_map=new_backup_replacements; // replacement_map.revert(); subtree_replacement_map=backup_subtree_replacements; @@ -1336,7 +1386,8 @@ namespace cadabra { } else { // txtout << tofind.node << "does not match" << std::endl; - replacement_map=backup_replacements; + if (swflag) Stopwatches["map_copies"].start(); + IF_TEST(replacement_map=backup_replacements;); new_replacement_map=new_backup_replacements; // replacement_map.revert(); subtree_replacement_map=backup_subtree_replacements; @@ -1353,8 +1404,9 @@ namespace cadabra { Ex::sibling_iterator st, Ex::iterator conditions) { - replacement_map_t backup_replacements(replacement_map); + IF_TEST(replacement_map_t backup_replacements(replacement_map);); // replacement_map.checkpoint(); + new_replacement_map_t backup_new_replacements(new_replacement_map); subtree_replacement_map_t backup_subtree_replacements(subtree_replacement_map); // 'Start' iterates over all terms, trying to find 'tofind'. It may happen that the @@ -1384,8 +1436,9 @@ namespace cadabra { } auto this_ratio = (*tofind->multiplier)/(*start->multiplier); if(this_ratio!=term_ratio) { - replacement_map=backup_replacements; + IF_TEST(replacement_map=backup_replacements;); // replacement_map.revert(); + new_replacement_map=backup_new_replacements; subtree_replacement_map=backup_subtree_replacements; } else { @@ -1398,8 +1451,9 @@ namespace cadabra { if(res==match_t::subtree_match) return res; else { factor_locations.pop_back(); - replacement_map=backup_replacements; + IF_TEST(replacement_map=backup_replacements;); // replacement_map.revert(); + new_replacement_map=backup_new_replacements; subtree_replacement_map=backup_subtree_replacements; } } @@ -1412,8 +1466,9 @@ namespace cadabra { } else { factor_locations.pop_back(); - replacement_map=backup_replacements; + IF_TEST(replacement_map=backup_replacements;); // replacement_map.revert(); + new_replacement_map=backup_new_replacements; subtree_replacement_map=backup_subtree_replacements; } } @@ -1421,8 +1476,9 @@ namespace cadabra { } else { // txtout << tofind.node << "does not match" << std::endl; - replacement_map=backup_replacements; + IF_TEST(replacement_map=backup_replacements;); // replacement_map.revert(); + new_replacement_map=backup_new_replacements; subtree_replacement_map=backup_subtree_replacements; } } @@ -2084,9 +2140,26 @@ namespace cadabra { bool lazy_less::operator()(const Lazy_Ex& one, const Lazy_Ex& two) const { return subtree_compare(nullptr, one, two, -2, true, 0, true) > 0; - } + } + Ex resolve_lazy(Lazy_Ex lazy) { + Ex ret {lazy.first}; + switch(lazy.second) { + case repl_t::erase_children: + ret.erase_children(ret.begin()); + break; + case repl_t::flip_parent_rel: + ret.begin()->flip_parent_rel(); + break; + case repl_t::erase_parent_rel: + ret.begin()->fl.parent_rel = str_node::parent_rel_t::p_none; + break; + case repl_t::same: + break; + } + return ret; + } } bool operator<(const cadabra::Ex::iterator& i1, const cadabra::Ex::iterator& i2) diff --git a/core/Compare.hh b/core/Compare.hh index 68cb4ee194..3e06623017 100644 --- a/core/Compare.hh +++ b/core/Compare.hh @@ -73,6 +73,8 @@ namespace cadabra { }; + Ex resolve_lazy(Lazy_Ex lazy); + int subtree_compare(const Properties*, Lazy_Ex one, Lazy_Ex two, int mod_prel=-2, bool checksets=true, int compare_multiplier=-2, diff --git a/core/algorithms/substitute.cc b/core/algorithms/substitute.cc index 2c8ec3d5e5..af35005e04 100644 --- a/core/algorithms/substitute.cc +++ b/core/algorithms/substitute.cc @@ -1,3 +1,17 @@ +#ifdef NDEBUG +# undef NDEBUG +#endif +#include + +// #define TEST + +#ifdef TEST + #define IF_TEST(...) __VA_ARGS__ +#else + #define IF_TEST(...) +#endif + + #include #include "Cleanup.hh" @@ -233,12 +247,11 @@ Algorithm::result_t substitute::apply(iterator& st) // NOTE: this does not yet insert the replacement into the main tree! iterator it=repl.begin(); - Ex_comparator::replacement_map_t::iterator loc; - Ex_comparator::subtree_replacement_map_t::iterator sloc; - - + IF_TEST(Ex_comparator::replacement_map_t::iterator loc;) Ex_comparator::new_replacement_map_t::iterator nloc; + Ex_comparator::subtree_replacement_map_t::iterator sloc; + std::vector subtree_insertion_points; while(it!=repl.end()) { bool is_stripped=false; @@ -246,20 +259,26 @@ Algorithm::result_t substitute::apply(iterator& st) // For some reason 'a?' is not found!?! Well, that's presumably because _{a?} does not // match ^{a?}. (though this does match when we write 'i' instead of a?. - loc=comparator.replacement_map.find(Ex(it)); nloc = comparator.new_replacement_map.find({it, repl_t::same}); - assert( (loc==comparator.replacement_map.end()) == (nloc==comparator.new_replacement_map.end()) ); - if(loc==comparator.replacement_map.end() && it->is_name_wildcard() && tr.number_of_children(it)!=0) { - Ex tmp(it); - tmp.erase_children(tmp.begin()); - loc=comparator.replacement_map.find(tmp); + IF_TEST( + loc=comparator.replacement_map.find(Ex(it)); + assert( (loc==comparator.replacement_map.end()) == (nloc==comparator.new_replacement_map.end()) ); + ) + + + if(nloc==comparator.new_replacement_map.end() && it->is_name_wildcard() && tr.number_of_children(it)!=0) { nloc=comparator.new_replacement_map.find({it, repl_t::erase_children}); is_stripped=true; + IF_TEST( + Ex tmp(it); + tmp.erase_children(tmp.begin()); + loc=comparator.replacement_map.find(tmp); + ) } //std::cerr << "consider element of repl " << Ex(it) << std::endl; - assert( (loc==comparator.replacement_map.end()) == (nloc==comparator.new_replacement_map.end()) ); - if(loc!=comparator.replacement_map.end()) { // name wildcards + IF_TEST( assert( (loc==comparator.replacement_map.end()) == (nloc==comparator.new_replacement_map.end()) ); ) + if(nloc!=comparator.new_replacement_map.end()) { // name wildcards DEBUGLN( std::cerr << "wildcard replaced: " << loc->first << " -> " << loc->second << std::endl; ) // When a replacement is made here, and the index is actually @@ -278,11 +297,19 @@ Algorithm::result_t substitute::apply(iterator& st) // TODO: should we replace brackets here too? DEBUGLN( std::cerr << "stripped replacing " << it << " with " << (*loc).second.begin() << " * " << *it->multiplier << std::endl; ); - it->name=(*loc).second.begin()->name; + it->name=nloc->second.first->name; // See the comment below about multipliers. - multiply(it->multiplier, *(*loc).second.begin()->multiplier); + multiply(it->multiplier, *nloc->second.first->multiplier); // assert((*loc).second.begin()->fl == (*nloc).second.first.begin()->fl); - it->fl=(*loc).second.begin()->fl; + it->fl= nloc->second.first->fl; + switch(nloc->second.second) { + case repl_t::erase_parent_rel: + it->fl.parent_rel = str_node::parent_rel_t::p_none; + break; + case repl_t::flip_parent_rel: + it->flip_parent_rel(); + break; + } } else { // Consider "A? -> 3 A?". If "A?" matches "2 C", then the replacement @@ -292,7 +319,8 @@ Algorithm::result_t substitute::apply(iterator& st) multiplier_t mt=*it->multiplier; DEBUGLN( std::cerr << "replacing " << it << " with " << (*loc).second.begin() << " * " << mt << std::endl; ); - it=tr.replace_index(it, (*loc).second.begin()); //, true); + // it=tr.replace_index(it, (*loc).second.begin()); //, true); + it=tr.replace_index(it, resolve_lazy(nloc->second).begin()); //, true); multiply(it->multiplier, mt); } it->fl.bracket=remember_br; From f5f60991f9680e23fee1e7da0a65603a500897a3 Mon Sep 17 00:00:00 2001 From: Daniel Butter Date: Mon, 4 Aug 2025 11:19:06 -0400 Subject: [PATCH 3/8] Removed old checkpoint_map comment. --- core/Compare.hh | 1 - 1 file changed, 1 deletion(-) diff --git a/core/Compare.hh b/core/Compare.hh index 3e06623017..d20793ae77 100644 --- a/core/Compare.hh +++ b/core/Compare.hh @@ -303,7 +303,6 @@ namespace cadabra { typedef std::map replacement_map_t; typedef std::map new_replacement_map_t; - // typedef checkpoint_map replacement_map_t; replacement_map_t replacement_map; new_replacement_map_t new_replacement_map; From 573e8ed594ee1b23386d9ca41b58243c0caa03bd Mon Sep 17 00:00:00 2001 From: Daniel Butter Date: Mon, 4 Aug 2025 12:00:10 -0400 Subject: [PATCH 4/8] Restructured Lazy_Ex as a class for better encapsulation. --- core/Compare.cc | 335 +++------------------------------- core/Compare.hh | 70 ++++--- core/algorithms/substitute.cc | 19 +- 3 files changed, 85 insertions(+), 339 deletions(-) diff --git a/core/Compare.cc b/core/Compare.cc index a85b453c4d..c2b5e0161d 100644 --- a/core/Compare.cc +++ b/core/Compare.cc @@ -39,264 +39,20 @@ namespace cadabra { int Ex_comparator::offset=0; - int subtree_compare(const Properties *properties, - Ex::iterator one, Ex::iterator two, - int mod_prel, bool, int compare_multiplier, bool literal_wildcards) + int subtree_compare(const Properties* properties, + const Lazy_Ex& one, const Lazy_Ex& two, + int mod_prel=-2, bool checksets=true, int compare_multiplier=-2, + bool literal_wildcards=false) { - // std::cerr << "comparing " << Ex(one) << " with " << Ex(two) << " " << mod_prel << ", " << checksets << std::endl; - - // The logic is to compare successive aspects of the two objects, returning a - // no-match code if a difference is found at a particular level, or continuing - // further down the line if there still is a match. - - // Compare multipliers. Skip this step if one of the objects is a rational and the - // other not, as in that case we are matching values to symbols. - if( one->is_rational()==two->is_rational() ) { - if(compare_multiplier==-2 && !two->is_name_wildcard() && !one->is_name_wildcard()) - if(one->multiplier != two->multiplier) { - if(*one->multiplier < *two->multiplier) return 2; - else return -2; - } - } - - int mult=2; - // Handle object wildcards and comparison - if(!literal_wildcards) { - if(one->is_object_wildcard() || two->is_object_wildcard()) - return 0; - } - - // Handle mismatching node names. - if(one->name!=two->name) { - // std::cerr << *one->name << " != " << *two->name << std::endl; - if(literal_wildcards) { - if(*one->name < *two->name) return mult; - else return -mult; - } - - DEBUGLN( std::cerr << "auto? " << one->is_autodeclare_wildcard() << ", " << two->is_numbered_symbol() << std::endl; ); - if( (one->is_autodeclare_wildcard() && two->is_numbered_symbol()) || - (two->is_autodeclare_wildcard() && one->is_numbered_symbol()) ) { - if( one->name_only() != two->name_only() ) { - if(*one->name < *two->name) return mult; - else return -mult; - } - } - else if( one->is_name_wildcard()==false && two->is_name_wildcard()==false ) { - if(*one->name < *two->name) return mult; - else return -mult; - } - - DEBUGLN( std::cerr << "match despite difference: " << *one->name << " and " << *two->name << std::endl; ); - } - - // Compare parent relations. - if(mod_prel<=-2) { - str_node::parent_rel_t p1=one->fl.parent_rel; - str_node::parent_rel_t p2=two->fl.parent_rel; - if(p1!=p2) { - return (p10) --mod_prel; - if(compare_multiplier==0) compare_multiplier=-2; - else if(compare_multiplier>0) --compare_multiplier; - - for(;;) { - Ex::sibling_iterator sib1=one.begin(), sib2=two.begin(); - - while(sib1!=one.end()) { - if(sib1->is_index() == do_indices) { - int ret=subtree_compare(properties, sib1,sib2, mod_prel, true /* checksets */, - compare_multiplier, literal_wildcards); - // std::cerr << "result " << ret << std::endl; - if(abs(ret)>1) - return ret/abs(ret)*mult; - if(ret!=0 && remember_ret==0) - remember_ret=ret; - } - ++sib1; - ++sib2; - } - - if(remember_ret!=0) break; - - if(!do_indices) do_indices=true; - else break; - } - - return remember_ret; + return subtree_compare(properties, one.it, two.it, mod_prel, checksets, compare_multiplier, literal_wildcards, one.op, two.op); } - int subtree_compare(const Properties *properties, - Lazy_Ex lazy_one, Lazy_Ex lazy_two, - int mod_prel, bool, int compare_multiplier, bool literal_wildcards) - { - Ex::iterator one = lazy_one.first; - Ex::iterator two = lazy_two.first; - - // std::cerr << "comparing " << Ex(one) << " with " << Ex(two) << " " << mod_prel << ", " << checksets << std::endl; - - // The logic is to compare successive aspects of the two objects, returning a - // no-match code if a difference is found at a particular level, or continuing - // further down the line if there still is a match. - - // Compare multipliers. Skip this step if one of the objects is a rational and the - // other not, as in that case we are matching values to symbols. - if( one->is_rational()==two->is_rational() ) { - if(compare_multiplier==-2 && !two->is_name_wildcard() && !one->is_name_wildcard()) - if(one->multiplier != two->multiplier) { - if(*one->multiplier < *two->multiplier) return 2; - else return -2; - } - } - - int mult=2; - // Handle object wildcards and comparison - if(!literal_wildcards) { - if(one->is_object_wildcard() || two->is_object_wildcard()) - return 0; - } - - // Handle mismatching node names. - if(one->name!=two->name) { - // std::cerr << *one->name << " != " << *two->name << std::endl; - if(literal_wildcards) { - if(*one->name < *two->name) return mult; - else return -mult; - } - - DEBUGLN( std::cerr << "auto? " << one->is_autodeclare_wildcard() << ", " << two->is_numbered_symbol() << std::endl; ); - if( (one->is_autodeclare_wildcard() && two->is_numbered_symbol()) || - (two->is_autodeclare_wildcard() && one->is_numbered_symbol()) ) { - if( one->name_only() != two->name_only() ) { - if(*one->name < *two->name) return mult; - else return -mult; - } - } - else if( one->is_name_wildcard()==false && two->is_name_wildcard()==false ) { - if(*one->name < *two->name) return mult; - else return -mult; - } - - DEBUGLN( std::cerr << "match despite difference: " << *one->name << " and " << *two->name << std::endl; ); - } - - // Compare parent relations. - if(mod_prel<=-2) { - str_node::parent_rel_t p1=one->fl.parent_rel; - str_node::parent_rel_t p2=two->fl.parent_rel; - - if (lazy_one.second == repl_t::erase_parent_rel) { - p1 = str_node::parent_rel_t::p_none; - } else if (lazy_one.second == repl_t::flip_parent_rel) { - if(p1==str_node::parent_rel_t::p_super) p1=str_node::parent_rel_t::p_sub; - else if(p1==str_node::parent_rel_t::p_sub) p1=str_node::parent_rel_t::p_super; - else throw std::logic_error("flip_parent_rel called on non-index"); - } - - if (lazy_two.second == repl_t::erase_parent_rel) { - p2 = str_node::parent_rel_t::p_none; - } else if (lazy_two.second == repl_t::flip_parent_rel) { - if(p2==str_node::parent_rel_t::p_super) p2=str_node::parent_rel_t::p_sub; - else if(p2==str_node::parent_rel_t::p_sub) p2=str_node::parent_rel_t::p_super; - else throw std::logic_error("flip_parent_rel called on non-index"); - } - - if(p1!=p2) { - return (p10) --mod_prel; - if(compare_multiplier==0) compare_multiplier=-2; - else if(compare_multiplier>0) --compare_multiplier; - - for(;;) { - Ex::sibling_iterator sib1=one.begin(), sib2=two.begin(); - - while(sib1!=one.end()) { - if(sib1->is_index() == do_indices) { - int ret=subtree_compare(properties, sib1,sib2, mod_prel, true /* checksets */, - compare_multiplier, literal_wildcards); - // std::cerr << "result " << ret << std::endl; - // FIXME(Dan): The below could be modified? - if(abs(ret)>1) - return ret/abs(ret)*mult; - if(ret!=0 && remember_ret==0) - remember_ret=ret; - } - ++sib1; - ++sib2; - } - - if(remember_ret!=0) break; - - if(!do_indices) do_indices=true; - else break; - } - - return remember_ret; - } int subtree_compare(const Properties *properties, - Lazy_Ex lazy_one, Lazy_Ex lazy_two, - int mod_prel, bool, int compare_multiplier, bool literal_wildcards) + Ex::iterator one, Ex::iterator two, + int mod_prel, bool, int compare_multiplier, bool literal_wildcards, + Lazy_Ex::repl_t op1, Lazy_Ex::repl_t op2) { - Ex::iterator one = lazy_one.first; - Ex::iterator two = lazy_two.first; - // std::cerr << "comparing " << Ex(one) << " with " << Ex(two) << " " << mod_prel << ", " << checksets << std::endl; // The logic is to compare successive aspects of the two objects, returning a @@ -349,17 +105,17 @@ namespace cadabra { str_node::parent_rel_t p1=one->fl.parent_rel; str_node::parent_rel_t p2=two->fl.parent_rel; - if (lazy_one.second == repl_t::erase_parent_rel) { + if (op1 == Lazy_Ex::repl_t::erase_parent_rel) { p1 = str_node::parent_rel_t::p_none; - } else if (lazy_one.second == repl_t::flip_parent_rel) { + } else if (op1 == Lazy_Ex::repl_t::flip_parent_rel) { if(p1==str_node::parent_rel_t::p_super) p1=str_node::parent_rel_t::p_sub; else if(p1==str_node::parent_rel_t::p_sub) p1=str_node::parent_rel_t::p_super; else throw std::logic_error("flip_parent_rel called on non-index"); } - if (lazy_two.second == repl_t::erase_parent_rel) { + if (op2 == Lazy_Ex::repl_t::erase_parent_rel) { p2 = str_node::parent_rel_t::p_none; - } else if (lazy_two.second == repl_t::flip_parent_rel) { + } else if (op2 == Lazy_Ex::repl_t::flip_parent_rel) { if(p2==str_node::parent_rel_t::p_super) p2=str_node::parent_rel_t::p_sub; else if(p2==str_node::parent_rel_t::p_sub) p2=str_node::parent_rel_t::p_super; else throw std::logic_error("flip_parent_rel called on non-index"); @@ -372,8 +128,8 @@ namespace cadabra { // Now turn to the child nodes. Before comparing them directly, first compare // the number of children, taking into account range wildcards. - int numch1 = (lazy_one.second == repl_t::erase_children) ? 0 : Ex::number_of_children(one); - int numch2 = (lazy_two.second == repl_t::erase_children) ? 0 : Ex::number_of_children(two); + int numch1 = (op1 == Lazy_Ex::repl_t::erase_children) ? 0 : Ex::number_of_children(one); + int numch2 = (op2 == Lazy_Ex::repl_t::erase_children) ? 0 : Ex::number_of_children(two); // FIXME: handle range wildcards as in Props.cc @@ -429,6 +185,7 @@ namespace cadabra { return remember_ret; } + bool tree_less(const Properties* properties, const Ex& one, const Ex& two, int mod_prel, bool checksets, int compare_multiplier) { return subtree_less(properties, one.begin(), two.begin(), mod_prel, checksets, compare_multiplier); @@ -892,7 +649,7 @@ namespace cadabra { IF_TEST(replacement_map_t::iterator loc=replacement_map.find(Ex(one));); // FIXME(Dan): Modified to work directly with iterator - new_replacement_map_t::iterator new_loc=new_replacement_map.find({one, repl_t::same}); + new_replacement_map_t::iterator new_loc=new_replacement_map.find({one, Lazy_Ex::repl_t::same}); bool tested_full=true; @@ -909,7 +666,7 @@ namespace cadabra { tmp1.erase_children(tmp1.begin()); loc = replacement_map.find(tmp1); ); - new_loc = new_replacement_map.find({one, repl_t::erase_children}); + new_loc = new_replacement_map.find({one, Lazy_Ex::repl_t::erase_children}); tested_full=false; } } @@ -924,11 +681,11 @@ namespace cadabra { // If this is an index/pattern, try to match the whole index/pattern. if(tested_full) { - cmp = subtree_compare(&properties, new_loc->second, {two, repl_t::same}, -2); + cmp = subtree_compare(&properties, new_loc->second, {two, Lazy_Ex::repl_t::same}, -2); IF_TEST(assert(cmp == subtree_compare(&properties, (*loc).second.begin(), two, -2));); } else { - cmp = subtree_compare(&properties, new_loc->second, {two, repl_t::erase_children}, -2); + cmp = subtree_compare(&properties, new_loc->second, {two, Lazy_Ex::repl_t::erase_children}, -2); IF_TEST ( Ex tmp2(two); tmp2.erase_children(tmp2.begin()); @@ -1055,7 +812,7 @@ namespace cadabra { // cadabra::one(tmp.begin()->multiplier); replacement_map[Ex(one)]=tmp; ); - new_replacement_map[{one, repl_t::same}] = {two, repl_t::same}; + new_replacement_map[{one, Lazy_Ex::repl_t::same}] = {two, Lazy_Ex::repl_t::same}; // if this is an index, also store the pattern with the parent_rel flipped @@ -1072,9 +829,9 @@ namespace cadabra { if(one->is_index()) { if(two->is_index()) - new_replacement_map[{one, repl_t::flip_parent_rel}] = {two, repl_t::flip_parent_rel}; + new_replacement_map[{one, Lazy_Ex::repl_t::flip_parent_rel}] = {two, Lazy_Ex::repl_t::flip_parent_rel}; else - new_replacement_map[{one, repl_t::flip_parent_rel}] = {two, repl_t::same}; + new_replacement_map[{one, Lazy_Ex::repl_t::flip_parent_rel}] = {two, Lazy_Ex::repl_t::same}; } // if this is a pattern and the pattern has a non-zero number of children, @@ -1086,12 +843,12 @@ namespace cadabra { tmp2.erase_children(tmp2.begin()); replacement_map[tmp1]=tmp2; ); - new_replacement_map[{one, repl_t::erase_children}]={two, repl_t::erase_children}; + new_replacement_map[{one, Lazy_Ex::repl_t::erase_children}]={two, Lazy_Ex::repl_t::erase_children}; } // and if this is a pattern also insert the one without the parent_rel if( /* one->is_name_wildcard() && */ one->is_index()) { //std::cerr << "storing pattern without pattern rel " << replacement_map.size() << std::endl; - auto new_fnd=new_replacement_map.find({one, repl_t::erase_parent_rel}); + auto new_fnd=new_replacement_map.find({one, Lazy_Ex::repl_t::erase_parent_rel}); IF_TEST( Ex tmp1(one), tmp2(two); tmp1.begin()->fl.parent_rel=str_node::p_none; @@ -1106,7 +863,7 @@ namespace cadabra { // assert(replacement_map.find(tmp1)!=replacement_map.end()); // std::cerr << "storing " << Ex(tmp1) << " -> " << Ex(tmp2) << std::endl; IF_TEST( replacement_map[tmp1]=tmp2; ); - new_replacement_map[{one, repl_t::erase_parent_rel}]={two, repl_t::erase_parent_rel}; + new_replacement_map[{one, Lazy_Ex::repl_t::erase_parent_rel}]={two, Lazy_Ex::repl_t::erase_parent_rel}; } DEBUGLN( std::cerr << "Replacement map is now:" << std::endl; @@ -1284,10 +1041,6 @@ namespace cadabra { Ex::sibling_iterator st, Ex::iterator conditions) { - bool swflag = !Stopwatches["internal"].stopped() & Stopwatches["can_move_adjacent"].stopped(); - if (swflag) Stopwatches["match_subproduct"].start(); - if (swflag) Stopwatches["map_copies"].start(); - IF_TEST(replacement_map_t backup_replacements(replacement_map);); new_replacement_map_t new_backup_replacements(new_replacement_map); @@ -1338,10 +1091,8 @@ namespace cadabra { DEBUGLN(std::cerr << "--- done can move ---" << sign << std::endl; ); } if(sign==0) { // object found, but we cannot move it in the right order - if (swflag) Stopwatches["map_copies"].start(); IF_TEST(replacement_map=backup_replacements;); new_replacement_map=new_backup_replacements; - // replacement_map.revert(); subtree_replacement_map=backup_subtree_replacements; } else { @@ -1358,10 +1109,8 @@ namespace cadabra { // txtout << tofind.node << "found factor useless " << start.node << std::endl; factor_locations.pop_back(); factor_moving_signs.pop_back(); - if (swflag) Stopwatches["map_copies"].start(); IF_TEST(replacement_map=backup_replacements;); new_replacement_map=new_backup_replacements; - // replacement_map.revert(); subtree_replacement_map=backup_subtree_replacements; } } @@ -1375,10 +1124,8 @@ namespace cadabra { else { factor_locations.pop_back(); factor_moving_signs.pop_back(); - if (swflag) Stopwatches["map_copies"].start(); IF_TEST(replacement_map=backup_replacements;); new_replacement_map=new_backup_replacements; - // replacement_map.revert(); subtree_replacement_map=backup_subtree_replacements; } } @@ -1386,10 +1133,8 @@ namespace cadabra { } else { // txtout << tofind.node << "does not match" << std::endl; - if (swflag) Stopwatches["map_copies"].start(); IF_TEST(replacement_map=backup_replacements;); new_replacement_map=new_backup_replacements; - // replacement_map.revert(); subtree_replacement_map=backup_subtree_replacements; } } @@ -1405,7 +1150,6 @@ namespace cadabra { Ex::iterator conditions) { IF_TEST(replacement_map_t backup_replacements(replacement_map);); - // replacement_map.checkpoint(); new_replacement_map_t backup_new_replacements(new_replacement_map); subtree_replacement_map_t backup_subtree_replacements(subtree_replacement_map); @@ -1437,7 +1181,6 @@ namespace cadabra { auto this_ratio = (*tofind->multiplier)/(*start->multiplier); if(this_ratio!=term_ratio) { IF_TEST(replacement_map=backup_replacements;); - // replacement_map.revert(); new_replacement_map=backup_new_replacements; subtree_replacement_map=backup_subtree_replacements; } @@ -1452,7 +1195,6 @@ namespace cadabra { else { factor_locations.pop_back(); IF_TEST(replacement_map=backup_replacements;); - // replacement_map.revert(); new_replacement_map=backup_new_replacements; subtree_replacement_map=backup_subtree_replacements; } @@ -1467,7 +1209,6 @@ namespace cadabra { else { factor_locations.pop_back(); IF_TEST(replacement_map=backup_replacements;); - // replacement_map.revert(); new_replacement_map=backup_new_replacements; subtree_replacement_map=backup_subtree_replacements; } @@ -1477,7 +1218,6 @@ namespace cadabra { else { // txtout << tofind.node << "does not match" << std::endl; IF_TEST(replacement_map=backup_replacements;); - // replacement_map.revert(); new_replacement_map=backup_new_replacements; subtree_replacement_map=backup_subtree_replacements; } @@ -2136,32 +1876,9 @@ namespace cadabra { if(ret < 0) return true; else return false; } - - - bool lazy_less::operator()(const Lazy_Ex& one, const Lazy_Ex& two) const { - return subtree_compare(nullptr, one, two, -2, true, 0, true) > 0; - } - - Ex resolve_lazy(Lazy_Ex lazy) { - Ex ret {lazy.first}; - - switch(lazy.second) { - case repl_t::erase_children: - ret.erase_children(ret.begin()); - break; - case repl_t::flip_parent_rel: - ret.begin()->flip_parent_rel(); - break; - case repl_t::erase_parent_rel: - ret.begin()->fl.parent_rel = str_node::parent_rel_t::p_none; - break; - case repl_t::same: - break; - } - return ret; - } } + bool operator<(const cadabra::Ex::iterator& i1, const cadabra::Ex::iterator& i2) { return i1.node < i2.node; diff --git a/core/Compare.hh b/core/Compare.hh index d20793ae77..5653d608e3 100644 --- a/core/Compare.hh +++ b/core/Compare.hh @@ -7,6 +7,49 @@ namespace cadabra { + // Lazy replacement type to avoid temporary Ex objects in compare methods. + class Lazy_Ex { + public: + enum class repl_t : std::uint8_t { + same, // identity operation + flip_parent_rel, // flip top node's parent_rel + erase_parent_rel, // set top node's parent_rel to p_none + erase_children // ignore any children + }; + + Lazy_Ex(Ex::iterator it_, repl_t op_) : it(it_), op(op_) {}; + + Ex::iterator it; + repl_t op; + + // Apply the operation only when needed. + Ex resolve() const noexcept { + Ex ret {it}; + switch(op) { + case repl_t::erase_children: + ret.erase_children(ret.begin()); + break; + case repl_t::flip_parent_rel: + ret.begin()->flip_parent_rel(); + break; + case repl_t::erase_parent_rel: + ret.begin()->fl.parent_rel = str_node::parent_rel_t::p_none; + break; + case repl_t::same: + break; + } + return ret; + } + + // Comparison routine used for Lazy_Ex replacement map + class less { + public: + bool operator()(const Lazy_Ex& first, const Lazy_Ex& second) const; + }; + + }; + + /// \ingroup compare /// /// Basic building block subtree comparison function for tensors @@ -56,32 +99,17 @@ namespace cadabra { int subtree_compare(const Properties*, Ex::iterator one, Ex::iterator two, int mod_prel=-2, bool checksets=true, int compare_multiplier=-2, - bool literal_wildcards=false); - - - - // Introduce a lazy replacement type to avoid temporary Ex objects - enum class repl_t {same, // identity operation - flip_parent_rel, // flip top node's parent_rel - erase_parent_rel, // erase top node's parent_rel - erase_children // ignore any children - }; - typedef std::pair Lazy_Ex; - class lazy_less { - public: - bool operator()(const Lazy_Ex& first, const Lazy_Ex& second) const; - - }; - - Ex resolve_lazy(Lazy_Ex lazy); + bool literal_wildcards=false, + Lazy_Ex::repl_t op1 = Lazy_Ex::repl_t::same, + Lazy_Ex::repl_t op2 = Lazy_Ex::repl_t::same); + // Revised subtree comparison to handle Lazy_Ex objects int subtree_compare(const Properties*, - Lazy_Ex one, Lazy_Ex two, + const Lazy_Ex& one, const Lazy_Ex& two, int mod_prel=-2, bool checksets=true, int compare_multiplier=-2, bool literal_wildcards=false); - /// Various comparison functions, some exact, some with pattern logic. bool tree_less(const Properties*, @@ -301,7 +329,7 @@ namespace cadabra { /// Map for the replacement of nodes (indices, patterns). typedef std::map replacement_map_t; - typedef std::map new_replacement_map_t; + typedef std::map new_replacement_map_t; replacement_map_t replacement_map; new_replacement_map_t new_replacement_map; diff --git a/core/algorithms/substitute.cc b/core/algorithms/substitute.cc index af35005e04..47ef74f2c7 100644 --- a/core/algorithms/substitute.cc +++ b/core/algorithms/substitute.cc @@ -259,7 +259,7 @@ Algorithm::result_t substitute::apply(iterator& st) // For some reason 'a?' is not found!?! Well, that's presumably because _{a?} does not // match ^{a?}. (though this does match when we write 'i' instead of a?. - nloc = comparator.new_replacement_map.find({it, repl_t::same}); + nloc = comparator.new_replacement_map.find({it, Lazy_Ex::repl_t::same}); IF_TEST( loc=comparator.replacement_map.find(Ex(it)); assert( (loc==comparator.replacement_map.end()) == (nloc==comparator.new_replacement_map.end()) ); @@ -267,7 +267,7 @@ Algorithm::result_t substitute::apply(iterator& st) if(nloc==comparator.new_replacement_map.end() && it->is_name_wildcard() && tr.number_of_children(it)!=0) { - nloc=comparator.new_replacement_map.find({it, repl_t::erase_children}); + nloc=comparator.new_replacement_map.find({it, Lazy_Ex::repl_t::erase_children}); is_stripped=true; IF_TEST( Ex tmp(it); @@ -297,16 +297,16 @@ Algorithm::result_t substitute::apply(iterator& st) // TODO: should we replace brackets here too? DEBUGLN( std::cerr << "stripped replacing " << it << " with " << (*loc).second.begin() << " * " << *it->multiplier << std::endl; ); - it->name=nloc->second.first->name; + it->name=nloc->second.it->name; // See the comment below about multipliers. - multiply(it->multiplier, *nloc->second.first->multiplier); + multiply(it->multiplier, *nloc->second.it->multiplier); // assert((*loc).second.begin()->fl == (*nloc).second.first.begin()->fl); - it->fl= nloc->second.first->fl; - switch(nloc->second.second) { - case repl_t::erase_parent_rel: + it->fl= nloc->second.it->fl; + switch(nloc->second.op) { + case Lazy_Ex::repl_t::erase_parent_rel: it->fl.parent_rel = str_node::parent_rel_t::p_none; break; - case repl_t::flip_parent_rel: + case Lazy_Ex::repl_t::flip_parent_rel: it->flip_parent_rel(); break; } @@ -320,7 +320,8 @@ Algorithm::result_t substitute::apply(iterator& st) DEBUGLN( std::cerr << "replacing " << it << " with " << (*loc).second.begin() << " * " << mt << std::endl; ); // it=tr.replace_index(it, (*loc).second.begin()); //, true); - it=tr.replace_index(it, resolve_lazy(nloc->second).begin()); //, true); + Ex nloc_resolved = nloc->second.resolve(); + it=tr.replace_index(it, nloc_resolved.begin()); //, true); multiply(it->multiplier, mt); } it->fl.bracket=remember_br; From c79e876cfb3606fcfad97cf1e7d74ab80bb3df51 Mon Sep 17 00:00:00 2001 From: Daniel Butter Date: Mon, 4 Aug 2025 14:00:45 -0400 Subject: [PATCH 5/8] Fixed small mistakes in Compare. Added size() to Ex object in Python for debugging. Added extra cell_id = 0 statement do console debugger would work in python. --- core/Compare.cc | 7 +++++-- core/Compare.hh | 3 ++- core/cadabra2_defaults.py.in | 3 +++ core/pythoncdb/py_ex.cc | 1 + 4 files changed, 11 insertions(+), 3 deletions(-) diff --git a/core/Compare.cc b/core/Compare.cc index c2b5e0161d..5428a3a861 100644 --- a/core/Compare.cc +++ b/core/Compare.cc @@ -41,8 +41,8 @@ namespace cadabra { int subtree_compare(const Properties* properties, const Lazy_Ex& one, const Lazy_Ex& two, - int mod_prel=-2, bool checksets=true, int compare_multiplier=-2, - bool literal_wildcards=false) + int mod_prel, bool checksets, int compare_multiplier, + bool literal_wildcards) { return subtree_compare(properties, one.it, two.it, mod_prel, checksets, compare_multiplier, literal_wildcards, one.op, two.op); } @@ -185,6 +185,9 @@ namespace cadabra { return remember_ret; } + bool Lazy_Ex::less::operator()(const Lazy_Ex& first, const Lazy_Ex& second) const { + return subtree_compare(nullptr, first.it, second.it, -2, true, 0, true, first.op, second.op) > 0; + } bool tree_less(const Properties* properties, const Ex& one, const Ex& two, int mod_prel, bool checksets, int compare_multiplier) { diff --git a/core/Compare.hh b/core/Compare.hh index 5653d608e3..c2d02f2364 100644 --- a/core/Compare.hh +++ b/core/Compare.hh @@ -17,8 +17,9 @@ namespace cadabra { erase_children // ignore any children }; + Lazy_Ex() = default; Lazy_Ex(Ex::iterator it_, repl_t op_) : it(it_), op(op_) {}; - + Ex::iterator it; repl_t op; diff --git a/core/cadabra2_defaults.py.in b/core/cadabra2_defaults.py.in index eccb76d03f..ef654a2041 100644 --- a/core/cadabra2_defaults.py.in +++ b/core/cadabra2_defaults.py.in @@ -550,6 +550,7 @@ class Console(object): """ def log(self, *objs): """Sends a string representation of objs to the console""" + cell_id = 0 if server.architecture() == "terminal": print(*objs) elif server.architecture() == "client-server": @@ -557,10 +558,12 @@ class Console(object): def clear(self): """Clears the output of the console window""" + cell_id = 0 if server.architecture() == "client-server": server.send("", "csl_clear", 0, cell_id, False) def warn(self, msg): + cell_id = 0 if server.architecture() == "terminal": print("Warning: " + msg) elif server.architecture() == "client-server": diff --git a/core/pythoncdb/py_ex.cc b/core/pythoncdb/py_ex.cc index 7376b302f4..930a9bf2cb 100644 --- a/core/pythoncdb/py_ex.cc +++ b/core/pythoncdb/py_ex.cc @@ -715,6 +715,7 @@ namespace cadabra { .def("state", &Ex::state) .def("reset", &Ex::reset_state) .def("copy", [](const Ex& ex) { return std::make_shared(ex); }) + .def("size", [](const Ex& ex) { return ex.size(); }) .def("changed", &Ex::changed_state) .def("cleanup", &Ex_cleanup) .def("__array__", [](Ex& ex) { From b35ba4af660ed4cf4a422df20bcbc47057c1fea0 Mon Sep 17 00:00:00 2001 From: Daniel Butter Date: Mon, 4 Aug 2025 14:57:47 -0400 Subject: [PATCH 6/8] Modified index_value_map as well. --- core/Compare.cc | 42 +++++++++++++++++++++++++++++-------- core/Compare.hh | 1 + core/algorithms/evaluate.cc | 14 +++++++------ 3 files changed, 42 insertions(+), 15 deletions(-) diff --git a/core/Compare.cc b/core/Compare.cc index 5428a3a861..9a771e8ff8 100644 --- a/core/Compare.cc +++ b/core/Compare.cc @@ -189,6 +189,11 @@ namespace cadabra { return subtree_compare(nullptr, first.it, second.it, -2, true, 0, true, first.op, second.op) > 0; } + bool operator==(const Lazy_Ex& first, const Lazy_Ex& second) { + return subtree_compare(nullptr, first.it, second.it, -2, true, 0, true, first.op, second.op) == 0; + } + + bool tree_less(const Properties* properties, const Ex& one, const Ex& two, int mod_prel, bool checksets, int compare_multiplier) { return subtree_less(properties, one.begin(), two.begin(), mod_prel, checksets, compare_multiplier); @@ -340,7 +345,8 @@ namespace cadabra { IF_TEST(replacement_map.clear();); new_replacement_map.clear(); subtree_replacement_map.clear(); - index_value_map.clear(); + IF_TEST(index_value_map.clear();) + new_index_value_map.clear(); factor_locations.clear(); factor_moving_signs.clear(); } @@ -949,21 +955,39 @@ namespace cadabra { if(ivals!=values.end()) { // Verify that the 'two' index has not already been matched to a value // different from 'one'. - Ex t1(two), t2(two), o1(one), o2(one); - t2.begin()->flip_parent_rel(); - o2.begin()->flip_parent_rel(); - auto prev1 = index_value_map.find(t1); - auto prev2 = index_value_map.find(t2); - if(prev1!=index_value_map.end() && ! (prev1->second==o1) ) { + auto nprev1 = new_index_value_map.find({two, Lazy_Ex::repl_t::same}); + auto nprev2 = new_index_value_map.find({two, Lazy_Ex::repl_t::flip_parent_rel}); + + Lazy_Ex new_o1 = {one, Lazy_Ex::repl_t::same}; + Lazy_Ex new_o2 = {one, Lazy_Ex::repl_t::flip_parent_rel}; + + IF_TEST( + Ex t1(two), t2(two), o1(one), o2(one); + t2.begin()->flip_parent_rel(); + o2.begin()->flip_parent_rel(); + auto prev1 = index_value_map.find(t1); + auto prev2 = index_value_map.find(t2); + + assert( (prev1==index_value_map.end()) == (nprev1 == new_index_value_map.end()) ); + assert( (prev2==index_value_map.end()) == (nprev2 == new_index_value_map.end()) ); + assert( (prev1->second == o1) == (nprev1->second == new_o1) ); + assert( (prev2->second == o2) == (nprev2->second == new_o2) ); + ) + + //if(prev1!=index_value_map.end() && ! (prev1->second==o1) ) { + if(nprev1!=new_index_value_map.end() && ! (nprev1->second==new_o1) ) { // std::cerr << "Previously 1 " << Ex(two) << " was " << Ex(prev1->second) << std::endl; return report(match_t::no_match_less); } - if(prev2!=index_value_map.end() && ! (prev2->second==o2) ) { + if(nprev2!=new_index_value_map.end() && ! (nprev2->second==new_o2) ) { // std::cerr << "Previously 2 " << Ex(two) << " was " << Ex(prev2->second) << std::endl; return report(match_t::no_match_less); } - index_value_map[Ex(two)]=Ex(one); + IF_TEST( + index_value_map[Ex(two)]=Ex(one); + ) + new_index_value_map[{two, Lazy_Ex::repl_t::same}] = {one, Lazy_Ex::repl_t::same}; return report(match_t::node_match); } else { diff --git a/core/Compare.hh b/core/Compare.hh index c2d02f2364..52869325a3 100644 --- a/core/Compare.hh +++ b/core/Compare.hh @@ -344,6 +344,7 @@ namespace cadabra { /// from replacement_map! replacement_map_t index_value_map; + new_replacement_map_t new_index_value_map; /// Information to keep track of where individual factors/terms /// in a sub-product/sub-sum were found, and (for sub-products) diff --git a/core/algorithms/evaluate.cc b/core/algorithms/evaluate.cc index de9f831b28..2c0af1efa6 100644 --- a/core/algorithms/evaluate.cc +++ b/core/algorithms/evaluate.cc @@ -346,12 +346,13 @@ Ex::iterator evaluate::handle_factor(sibling_iterator sib, const IndexClassifier fi=ind_free.begin(); while(fi!=ind_free.end()) { - for(auto& r: subs.comparator.index_value_map) { - if(fi->first == r.first) { + for(auto& r: subs.comparator.new_index_value_map) { + if(fi->first == r.first.resolve()) { #ifdef DEBUG // std::cerr << "adding " << r.second.begin() << std::endl; #endif - repl.append_child(il, r.second.begin())->fl.parent_rel=str_node::p_none; + Ex lazy_resolved = r.second.resolve(); + repl.append_child(il, lazy_resolved.begin())->fl.parent_rel=str_node::p_none; break; } } @@ -362,12 +363,13 @@ Ex::iterator evaluate::handle_factor(sibling_iterator sib, const IndexClassifier } else { while(fi!=full_ind_free.end()) { - for(auto& r: subs.comparator.index_value_map) { - if(fi->first == r.first) { + for(auto& r: subs.comparator.new_index_value_map) { + if(fi->first == r.first.resolve()) { #ifdef DEBUG // std::cerr << "adding2 " << r.second.begin() << std::endl; #endif - repl.append_child(il, r.second.begin())->fl.parent_rel=str_node::p_none; + Ex lazy_resolved = r.second.resolve(); + repl.append_child(il, lazy_resolved.begin())->fl.parent_rel=str_node::p_none; break; } } From 67856248da9001bea1544cd6f0d1c7a972bb9a91 Mon Sep 17 00:00:00 2001 From: Daniel Butter Date: Wed, 6 Aug 2025 19:24:25 -0400 Subject: [PATCH 7/8] Added new replacement maps to conditional compare. After passing testing, removed all testing structure and renamed new_replacement_map to replacement_map. --- core/Compare.cc | 159 ++++++++-------------------------- core/Compare.hh | 11 +-- core/algorithms/substitute.cc | 37 ++------ 3 files changed, 43 insertions(+), 164 deletions(-) diff --git a/core/Compare.cc b/core/Compare.cc index 9a771e8ff8..c9250fe969 100644 --- a/core/Compare.cc +++ b/core/Compare.cc @@ -1,20 +1,3 @@ -#ifdef NDEBUG -# undef NDEBUG -#endif -#include - -// #define TEST - -#ifdef TEST - #define IF_TEST(...) __VA_ARGS__ -#else - #define IF_TEST(...) -#endif - - - -#include - #include "Compare.hh" #include "Algorithm.hh" // FIXME: only needed because index_iterator is in there #include "Exceptions.hh" @@ -31,6 +14,7 @@ #include "properties/DiracBar.hh" #include "properties/Integer.hh" #include "properties/SortOrder.hh" +#include // #define DEBUG "Compare.cc" #include "Debug.hh" @@ -166,9 +150,12 @@ namespace cadabra { int ret=subtree_compare(properties, sib1,sib2, mod_prel, true /* checksets */, compare_multiplier, literal_wildcards); // std::cerr << "result " << ret << std::endl; - // FIXME(Dan): The below could be modified? + /* if(abs(ret)>1) return ret/abs(ret)*mult; + */ + if (ret>1) return mult; + if (ret<-1) return -mult; if(ret!=0 && remember_ret==0) remember_ret=ret; } @@ -342,10 +329,8 @@ namespace cadabra { void Ex_comparator::clear() { - IF_TEST(replacement_map.clear();); - new_replacement_map.clear(); + replacement_map.clear(); subtree_replacement_map.clear(); - IF_TEST(index_value_map.clear();) new_index_value_map.clear(); factor_locations.clear(); factor_moving_signs.clear(); @@ -655,10 +640,9 @@ namespace cadabra { // triggering a rule for an upper index, we simply store both rules (see // below) so that searching for rules can remain simple. - IF_TEST(replacement_map_t::iterator loc=replacement_map.find(Ex(one));); // FIXME(Dan): Modified to work directly with iterator - new_replacement_map_t::iterator new_loc=new_replacement_map.find({one, Lazy_Ex::repl_t::same}); + replacement_map_t::iterator new_loc=replacement_map.find({one, Lazy_Ex::repl_t::same}); bool tested_full=true; @@ -666,22 +650,15 @@ namespace cadabra { // also search the pattern without the children (but not if // this node is an Accent, because then the child nodes are // very relevant). - IF_TEST(assert( (new_loc == new_replacement_map.end()) == (loc == replacement_map.end()) ); ); - if(new_loc == new_replacement_map.end() && Ex::number_of_children(one)!=0) { + if(new_loc == replacement_map.end() && Ex::number_of_children(one)!=0) { DEBUGLN( std::cerr << tab() << "**** not found, trying without child nodes" << std::endl; ); if(properties.get(one)==0 ) { - IF_TEST ( - Ex tmp1(one); - tmp1.erase_children(tmp1.begin()); - loc = replacement_map.find(tmp1); - ); - new_loc = new_replacement_map.find({one, Lazy_Ex::repl_t::erase_children}); + new_loc = replacement_map.find({one, Lazy_Ex::repl_t::erase_children}); tested_full=false; } } - IF_TEST( assert( (new_loc == new_replacement_map.end()) == (loc == replacement_map.end()) ); ); - if(new_loc != new_replacement_map.end()) { + if(new_loc != replacement_map.end()) { // We constructed a replacement rule for this node already at an earlier // stage. Need to make sure that that rule is consistent with what we // found now. @@ -691,15 +668,9 @@ namespace cadabra { // If this is an index/pattern, try to match the whole index/pattern. if(tested_full) { cmp = subtree_compare(&properties, new_loc->second, {two, Lazy_Ex::repl_t::same}, -2); - IF_TEST(assert(cmp == subtree_compare(&properties, (*loc).second.begin(), two, -2));); } else { cmp = subtree_compare(&properties, new_loc->second, {two, Lazy_Ex::repl_t::erase_children}, -2); - IF_TEST ( - Ex tmp2(two); - tmp2.erase_children(tmp2.begin()); - assert(cmp==subtree_compare(&properties, (*loc).second.begin(), tmp2.begin(), -2)); - ); } DEBUGLN(std::cerr << " pattern " << two << " should be " << (*loc).second.begin() @@ -816,63 +787,32 @@ namespace cadabra { // We absolutely need to keep the multiplier here. If we don't, then it // becomes very messy to restore multipliers of child nodes in the pattern, // e.g A?_{m n} matching Q_{1 -1}. Do not set the multiplier to one! - IF_TEST( - Ex tmp(two); - // cadabra::one(tmp.begin()->multiplier); - replacement_map[Ex(one)]=tmp; - ); - new_replacement_map[{one, Lazy_Ex::repl_t::same}] = {two, Lazy_Ex::repl_t::same}; + replacement_map[{one, Lazy_Ex::repl_t::same}] = {two, Lazy_Ex::repl_t::same}; // if this is an index, also store the pattern with the parent_rel flipped - IF_TEST( - if(one->is_index()) { - Ex cmptree1(one); - Ex cmptree2(two); - cmptree1.begin()->flip_parent_rel(); - if(two->is_index()) - cmptree2.begin()->flip_parent_rel(); - replacement_map[cmptree1]=cmptree2; - } - ); - if(one->is_index()) { if(two->is_index()) - new_replacement_map[{one, Lazy_Ex::repl_t::flip_parent_rel}] = {two, Lazy_Ex::repl_t::flip_parent_rel}; + replacement_map[{one, Lazy_Ex::repl_t::flip_parent_rel}] = {two, Lazy_Ex::repl_t::flip_parent_rel}; else - new_replacement_map[{one, Lazy_Ex::repl_t::flip_parent_rel}] = {two, Lazy_Ex::repl_t::same}; + replacement_map[{one, Lazy_Ex::repl_t::flip_parent_rel}] = {two, Lazy_Ex::repl_t::same}; } // if this is a pattern and the pattern has a non-zero number of children, // also add the pattern without the children if(Ex::number_of_children(one)!=0) { - IF_TEST( - Ex tmp1(one), tmp2(two); - tmp1.erase_children(tmp1.begin()); - tmp2.erase_children(tmp2.begin()); - replacement_map[tmp1]=tmp2; - ); - new_replacement_map[{one, Lazy_Ex::repl_t::erase_children}]={two, Lazy_Ex::repl_t::erase_children}; + replacement_map[{one, Lazy_Ex::repl_t::erase_children}]={two, Lazy_Ex::repl_t::erase_children}; } // and if this is a pattern also insert the one without the parent_rel if( /* one->is_name_wildcard() && */ one->is_index()) { //std::cerr << "storing pattern without pattern rel " << replacement_map.size() << std::endl; - auto new_fnd=new_replacement_map.find({one, Lazy_Ex::repl_t::erase_parent_rel}); - IF_TEST( - Ex tmp1(one), tmp2(two); - tmp1.begin()->fl.parent_rel=str_node::p_none; - tmp2.begin()->fl.parent_rel=str_node::p_none; - // We do not want this pattern to be present already. - auto fnd=replacement_map.find(tmp1); - assert( (fnd==replacement_map.end()) == (new_fnd==new_replacement_map.end())); - ); - if(new_fnd!=new_replacement_map.end()) { + auto new_fnd=replacement_map.find({one, Lazy_Ex::repl_t::erase_parent_rel}); + if(new_fnd!=replacement_map.end()) { throw InternalError("Attempting to duplicate replacement rule."); } // assert(replacement_map.find(tmp1)!=replacement_map.end()); // std::cerr << "storing " << Ex(tmp1) << " -> " << Ex(tmp2) << std::endl; - IF_TEST( replacement_map[tmp1]=tmp2; ); - new_replacement_map[{one, Lazy_Ex::repl_t::erase_parent_rel}]={two, Lazy_Ex::repl_t::erase_parent_rel}; + replacement_map[{one, Lazy_Ex::repl_t::erase_parent_rel}]={two, Lazy_Ex::repl_t::erase_parent_rel}; } DEBUGLN( std::cerr << "Replacement map is now:" << std::endl; @@ -961,19 +901,6 @@ namespace cadabra { Lazy_Ex new_o1 = {one, Lazy_Ex::repl_t::same}; Lazy_Ex new_o2 = {one, Lazy_Ex::repl_t::flip_parent_rel}; - IF_TEST( - Ex t1(two), t2(two), o1(one), o2(one); - t2.begin()->flip_parent_rel(); - o2.begin()->flip_parent_rel(); - auto prev1 = index_value_map.find(t1); - auto prev2 = index_value_map.find(t2); - - assert( (prev1==index_value_map.end()) == (nprev1 == new_index_value_map.end()) ); - assert( (prev2==index_value_map.end()) == (nprev2 == new_index_value_map.end()) ); - assert( (prev1->second == o1) == (nprev1->second == new_o1) ); - assert( (prev2->second == o2) == (nprev2->second == new_o2) ); - ) - //if(prev1!=index_value_map.end() && ! (prev1->second==o1) ) { if(nprev1!=new_index_value_map.end() && ! (nprev1->second==new_o1) ) { // std::cerr << "Previously 1 " << Ex(two) << " was " << Ex(prev1->second) << std::endl; @@ -984,9 +911,6 @@ namespace cadabra { return report(match_t::no_match_less); } - IF_TEST( - index_value_map[Ex(two)]=Ex(one); - ) new_index_value_map[{two, Lazy_Ex::repl_t::same}] = {one, Lazy_Ex::repl_t::same}; return report(match_t::node_match); } @@ -1068,9 +992,8 @@ namespace cadabra { Ex::sibling_iterator st, Ex::iterator conditions) { - IF_TEST(replacement_map_t backup_replacements(replacement_map);); - new_replacement_map_t new_backup_replacements(new_replacement_map); + replacement_map_t new_backup_replacements(replacement_map); subtree_replacement_map_t backup_subtree_replacements(subtree_replacement_map); // 'Start' iterates over all factors, trying to find 'tofind'. It may happen that the @@ -1118,8 +1041,7 @@ namespace cadabra { DEBUGLN(std::cerr << "--- done can move ---" << sign << std::endl; ); } if(sign==0) { // object found, but we cannot move it in the right order - IF_TEST(replacement_map=backup_replacements;); - new_replacement_map=new_backup_replacements; + replacement_map=new_backup_replacements; subtree_replacement_map=backup_subtree_replacements; } else { @@ -1136,8 +1058,7 @@ namespace cadabra { // txtout << tofind.node << "found factor useless " << start.node << std::endl; factor_locations.pop_back(); factor_moving_signs.pop_back(); - IF_TEST(replacement_map=backup_replacements;); - new_replacement_map=new_backup_replacements; + replacement_map=new_backup_replacements; subtree_replacement_map=backup_subtree_replacements; } } @@ -1151,8 +1072,7 @@ namespace cadabra { else { factor_locations.pop_back(); factor_moving_signs.pop_back(); - IF_TEST(replacement_map=backup_replacements;); - new_replacement_map=new_backup_replacements; + replacement_map=new_backup_replacements; subtree_replacement_map=backup_subtree_replacements; } } @@ -1160,8 +1080,7 @@ namespace cadabra { } else { // txtout << tofind.node << "does not match" << std::endl; - IF_TEST(replacement_map=backup_replacements;); - new_replacement_map=new_backup_replacements; + replacement_map=new_backup_replacements; subtree_replacement_map=backup_subtree_replacements; } } @@ -1176,8 +1095,7 @@ namespace cadabra { Ex::sibling_iterator st, Ex::iterator conditions) { - IF_TEST(replacement_map_t backup_replacements(replacement_map);); - new_replacement_map_t backup_new_replacements(new_replacement_map); + replacement_map_t backup_new_replacements(replacement_map); subtree_replacement_map_t backup_subtree_replacements(subtree_replacement_map); // 'Start' iterates over all terms, trying to find 'tofind'. It may happen that the @@ -1207,8 +1125,7 @@ namespace cadabra { } auto this_ratio = (*tofind->multiplier)/(*start->multiplier); if(this_ratio!=term_ratio) { - IF_TEST(replacement_map=backup_replacements;); - new_replacement_map=backup_new_replacements; + replacement_map=backup_new_replacements; subtree_replacement_map=backup_subtree_replacements; } else { @@ -1221,8 +1138,7 @@ namespace cadabra { if(res==match_t::subtree_match) return res; else { factor_locations.pop_back(); - IF_TEST(replacement_map=backup_replacements;); - new_replacement_map=backup_new_replacements; + replacement_map=backup_new_replacements; subtree_replacement_map=backup_subtree_replacements; } } @@ -1235,8 +1151,7 @@ namespace cadabra { } else { factor_locations.pop_back(); - IF_TEST(replacement_map=backup_replacements;); - new_replacement_map=backup_new_replacements; + replacement_map=backup_new_replacements; subtree_replacement_map=backup_subtree_replacements; } } @@ -1244,8 +1159,7 @@ namespace cadabra { } else { // txtout << tofind.node << "does not match" << std::endl; - IF_TEST(replacement_map=backup_replacements;); - new_replacement_map=backup_new_replacements; + replacement_map=backup_new_replacements; subtree_replacement_map=backup_subtree_replacements; } } @@ -1747,11 +1661,11 @@ namespace cadabra { // those rules give a different result. But first check that there are rules // to start with. // std::cerr << *lhs->name << " !=? " << *rhs->name << std::endl; - if(replacement_map.find(Ex(lhs))==replacement_map.end() || - replacement_map.find(Ex(rhs))==replacement_map.end()) return true; + if(replacement_map.find({lhs, Lazy_Ex::repl_t::same})==replacement_map.end() || + replacement_map.find({rhs, Lazy_Ex::repl_t::same})==replacement_map.end()) return true; // std::cerr << *lhs->name << " !=?? " << *rhs->name << std::endl; // FIXME(Dan): Above and Below need to be modified - if(tree_exact_equal(&properties, replacement_map[Ex(lhs)], replacement_map[Ex(rhs)])) { + if(tree_exact_equal(&properties, replacement_map[{lhs, Lazy_Ex::repl_t::same}].resolve(), replacement_map[{rhs, Lazy_Ex::repl_t::same}].resolve())) { return false; } } @@ -1762,10 +1676,10 @@ namespace cadabra { multiplier_t mlhs; if(lhs->is_rational()==false) { - auto fnd=replacement_map.find(Ex(lhs)); + auto fnd=replacement_map.find({lhs, Lazy_Ex::repl_t::same}); // std::cerr << "finding " << lhs << std::endl; if(fnd!=replacement_map.end()) { - auto tn=fnd->second.begin(); + auto tn=fnd->second.resolve().begin(); // std::cerr << tn << std::endl; if(tn->is_rational()) mlhs=*tn->multiplier; @@ -1784,9 +1698,9 @@ namespace cadabra { // FIXME: abstract into Storage multiplier_t mrhs; if(rhs->is_rational()==false) { - auto fnd=replacement_map.find(Ex(rhs)); + auto fnd=replacement_map.find({rhs, Lazy_Ex::repl_t::same}); if(fnd!=replacement_map.end()) { - auto tn=fnd->second.begin(); + auto tn=fnd->second.resolve().begin(); if(tn->is_rational()) mrhs=*tn->multiplier; else { @@ -1811,7 +1725,7 @@ namespace cadabra { it2=it; ++it2; while(it2!=replacement_map.end()) { - if(tree_exact_equal(&properties, it->second, it2->second)) { + if(tree_exact_equal(&properties, it->second.resolve(), it2->second.resolve())) { ++countpairs; break; } @@ -1833,8 +1747,7 @@ namespace cadabra { // txtout << "matching " << *comp.replacement_map[lhs->name] // << " with pattern " << pat << std::endl; std::regex reg(pat); - // FIXME(Dan): Below - if (!std::regex_match(*(replacement_map[Ex(lhs)].begin()->name), reg)) + if (!std::regex_match(*(replacement_map[{lhs, Lazy_Ex::repl_t::same}].resolve().begin()->name), reg)) return false; } // V2: FIXME: re-enable searching for properties diff --git a/core/Compare.hh b/core/Compare.hh index 52869325a3..0775099ef4 100644 --- a/core/Compare.hh +++ b/core/Compare.hh @@ -328,23 +328,16 @@ namespace cadabra { bool satisfies_conditions(Ex::iterator conditions, std::string& error); /// Map for the replacement of nodes (indices, patterns). - - typedef std::map replacement_map_t; - typedef std::map new_replacement_map_t; - + typedef std::map replacement_map_t; replacement_map_t replacement_map; - new_replacement_map_t new_replacement_map; /// Map for the replacement of entire subtrees (object patterns). - typedef std::map subtree_replacement_map_t; subtree_replacement_map_t subtree_replacement_map; /// Map for matching of index names to index values. Note: this is in the opposite order /// from replacement_map! - - replacement_map_t index_value_map; - new_replacement_map_t new_index_value_map; + replacement_map_t new_index_value_map; /// Information to keep track of where individual factors/terms /// in a sub-product/sub-sum were found, and (for sub-products) diff --git a/core/algorithms/substitute.cc b/core/algorithms/substitute.cc index 47ef74f2c7..6381661542 100644 --- a/core/algorithms/substitute.cc +++ b/core/algorithms/substitute.cc @@ -1,18 +1,3 @@ -#ifdef NDEBUG -# undef NDEBUG -#endif -#include - -// #define TEST - -#ifdef TEST - #define IF_TEST(...) __VA_ARGS__ -#else - #define IF_TEST(...) -#endif - - - #include #include "Cleanup.hh" #include "Exceptions.hh" @@ -247,8 +232,7 @@ Algorithm::result_t substitute::apply(iterator& st) // NOTE: this does not yet insert the replacement into the main tree! iterator it=repl.begin(); - IF_TEST(Ex_comparator::replacement_map_t::iterator loc;) - Ex_comparator::new_replacement_map_t::iterator nloc; + Ex_comparator::replacement_map_t::iterator nloc; Ex_comparator::subtree_replacement_map_t::iterator sloc; @@ -259,26 +243,15 @@ Algorithm::result_t substitute::apply(iterator& st) // For some reason 'a?' is not found!?! Well, that's presumably because _{a?} does not // match ^{a?}. (though this does match when we write 'i' instead of a?. - nloc = comparator.new_replacement_map.find({it, Lazy_Ex::repl_t::same}); - IF_TEST( - loc=comparator.replacement_map.find(Ex(it)); - assert( (loc==comparator.replacement_map.end()) == (nloc==comparator.new_replacement_map.end()) ); - ) - + nloc = comparator.replacement_map.find({it, Lazy_Ex::repl_t::same}); - if(nloc==comparator.new_replacement_map.end() && it->is_name_wildcard() && tr.number_of_children(it)!=0) { - nloc=comparator.new_replacement_map.find({it, Lazy_Ex::repl_t::erase_children}); + if(nloc==comparator.replacement_map.end() && it->is_name_wildcard() && tr.number_of_children(it)!=0) { + nloc=comparator.replacement_map.find({it, Lazy_Ex::repl_t::erase_children}); is_stripped=true; - IF_TEST( - Ex tmp(it); - tmp.erase_children(tmp.begin()); - loc=comparator.replacement_map.find(tmp); - ) } //std::cerr << "consider element of repl " << Ex(it) << std::endl; - IF_TEST( assert( (loc==comparator.replacement_map.end()) == (nloc==comparator.new_replacement_map.end()) ); ) - if(nloc!=comparator.new_replacement_map.end()) { // name wildcards + if(nloc!=comparator.replacement_map.end()) { // name wildcards DEBUGLN( std::cerr << "wildcard replaced: " << loc->first << " -> " << loc->second << std::endl; ) // When a replacement is made here, and the index is actually From f0d7bd28dc51f3426fa21fc35cb1c55e4af2497d Mon Sep 17 00:00:00 2001 From: Daniel Butter Date: Fri, 15 Aug 2025 12:11:22 -0400 Subject: [PATCH 8/8] Removed "new" prefix from some replacement_map variables. --- core/Compare.cc | 22 +++++++++++----------- core/Compare.hh | 2 +- core/algorithms/evaluate.cc | 4 ++-- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/core/Compare.cc b/core/Compare.cc index c9250fe969..4b3dfd40f2 100644 --- a/core/Compare.cc +++ b/core/Compare.cc @@ -331,7 +331,7 @@ namespace cadabra { { replacement_map.clear(); subtree_replacement_map.clear(); - new_index_value_map.clear(); + index_value_map.clear(); factor_locations.clear(); factor_moving_signs.clear(); } @@ -895,23 +895,23 @@ namespace cadabra { if(ivals!=values.end()) { // Verify that the 'two' index has not already been matched to a value // different from 'one'. - auto nprev1 = new_index_value_map.find({two, Lazy_Ex::repl_t::same}); - auto nprev2 = new_index_value_map.find({two, Lazy_Ex::repl_t::flip_parent_rel}); + auto nprev1 = index_value_map.find({two, Lazy_Ex::repl_t::same}); + auto nprev2 = index_value_map.find({two, Lazy_Ex::repl_t::flip_parent_rel}); Lazy_Ex new_o1 = {one, Lazy_Ex::repl_t::same}; Lazy_Ex new_o2 = {one, Lazy_Ex::repl_t::flip_parent_rel}; //if(prev1!=index_value_map.end() && ! (prev1->second==o1) ) { - if(nprev1!=new_index_value_map.end() && ! (nprev1->second==new_o1) ) { + if(nprev1!=index_value_map.end() && ! (nprev1->second==new_o1) ) { // std::cerr << "Previously 1 " << Ex(two) << " was " << Ex(prev1->second) << std::endl; return report(match_t::no_match_less); } - if(nprev2!=new_index_value_map.end() && ! (nprev2->second==new_o2) ) { + if(nprev2!=index_value_map.end() && ! (nprev2->second==new_o2) ) { // std::cerr << "Previously 2 " << Ex(two) << " was " << Ex(prev2->second) << std::endl; return report(match_t::no_match_less); } - new_index_value_map[{two, Lazy_Ex::repl_t::same}] = {one, Lazy_Ex::repl_t::same}; + index_value_map[{two, Lazy_Ex::repl_t::same}] = {one, Lazy_Ex::repl_t::same}; return report(match_t::node_match); } else { @@ -1095,7 +1095,7 @@ namespace cadabra { Ex::sibling_iterator st, Ex::iterator conditions) { - replacement_map_t backup_new_replacements(replacement_map); + replacement_map_t backup_replacements(replacement_map); subtree_replacement_map_t backup_subtree_replacements(subtree_replacement_map); // 'Start' iterates over all terms, trying to find 'tofind'. It may happen that the @@ -1125,7 +1125,7 @@ namespace cadabra { } auto this_ratio = (*tofind->multiplier)/(*start->multiplier); if(this_ratio!=term_ratio) { - replacement_map=backup_new_replacements; + replacement_map=backup_replacements; subtree_replacement_map=backup_subtree_replacements; } else { @@ -1138,7 +1138,7 @@ namespace cadabra { if(res==match_t::subtree_match) return res; else { factor_locations.pop_back(); - replacement_map=backup_new_replacements; + replacement_map=backup_replacements; subtree_replacement_map=backup_subtree_replacements; } } @@ -1151,7 +1151,7 @@ namespace cadabra { } else { factor_locations.pop_back(); - replacement_map=backup_new_replacements; + replacement_map=backup_replacements; subtree_replacement_map=backup_subtree_replacements; } } @@ -1159,7 +1159,7 @@ namespace cadabra { } else { // txtout << tofind.node << "does not match" << std::endl; - replacement_map=backup_new_replacements; + replacement_map=backup_replacements; subtree_replacement_map=backup_subtree_replacements; } } diff --git a/core/Compare.hh b/core/Compare.hh index 0775099ef4..524ae4c031 100644 --- a/core/Compare.hh +++ b/core/Compare.hh @@ -337,7 +337,7 @@ namespace cadabra { /// Map for matching of index names to index values. Note: this is in the opposite order /// from replacement_map! - replacement_map_t new_index_value_map; + replacement_map_t index_value_map; /// Information to keep track of where individual factors/terms /// in a sub-product/sub-sum were found, and (for sub-products) diff --git a/core/algorithms/evaluate.cc b/core/algorithms/evaluate.cc index 2c0af1efa6..0665dbaa9e 100644 --- a/core/algorithms/evaluate.cc +++ b/core/algorithms/evaluate.cc @@ -346,7 +346,7 @@ Ex::iterator evaluate::handle_factor(sibling_iterator sib, const IndexClassifier fi=ind_free.begin(); while(fi!=ind_free.end()) { - for(auto& r: subs.comparator.new_index_value_map) { + for(auto& r: subs.comparator.index_value_map) { if(fi->first == r.first.resolve()) { #ifdef DEBUG // std::cerr << "adding " << r.second.begin() << std::endl; @@ -363,7 +363,7 @@ Ex::iterator evaluate::handle_factor(sibling_iterator sib, const IndexClassifier } else { while(fi!=full_ind_free.end()) { - for(auto& r: subs.comparator.new_index_value_map) { + for(auto& r: subs.comparator.index_value_map) { if(fi->first == r.first.resolve()) { #ifdef DEBUG // std::cerr << "adding2 " << r.second.begin() << std::endl;