Hello! On Sun, Oct 21, 2007 at 01:26:45PM +0100, Vladimir V. Kisil wrote:
Actually, the patch introduces problems which are not specific to clifford.cpp. In the self-check errors start from color.cpp, see the relevant portion of check/exams.out file below. Could your patch breaks simplification of indexed object somehow?
Yep. That patch is somewhat insane, here is the [more] correct (i.e. `make check' does not report any errors) one: [PATCH] Attempt to fix ncmul performance regression due to dummy indices renaming. Do not rename dummy indices if the object has no indices at all. --- ginac/ncmul.cpp | 36 ++++++++++++++++++++++-------------- 1 file changed, 22 insertions(+), 14 deletions(-) diff --git a/ginac/ncmul.cpp b/ginac/ncmul.cpp index 3712329..34f2c7b 100644 --- a/ginac/ncmul.cpp +++ b/ginac/ncmul.cpp @@ -124,18 +124,19 @@ bool ncmul::info(unsigned inf) const return inherited::info(inf); } -typedef std::vector<int> intvector; +typedef std::vector<std::size_t> uintvector; ex ncmul::expand(unsigned options) const { + const bool skip_idx_rename = ! info(info_flags::has_indices); // First, expand the children std::auto_ptr<exvector> vp = expandchildren(options); const exvector &expanded_seq = vp.get() ? *vp : this->seq; // Now, look for all the factors that are sums and remember their // position and number of terms. - intvector positions_of_adds(expanded_seq.size()); - intvector number_of_add_operands(expanded_seq.size()); + uintvector positions_of_adds(expanded_seq.size()); + uintvector number_of_add_operands(expanded_seq.size()); size_t number_of_adds = 0; size_t number_of_expanded_terms = 1; @@ -167,7 +168,7 @@ ex ncmul::expand(unsigned options) const exvector distrseq; distrseq.reserve(number_of_expanded_terms); - intvector k(number_of_adds); + uintvector k(number_of_adds); /* Rename indices in the static members of the product */ exvector expanded_seq_mod; @@ -176,30 +177,37 @@ ex ncmul::expand(unsigned options) const for (size_t i=0; i<expanded_seq.size(); i++) { if (i == positions_of_adds[j]) { + // XXX: could anyone explain why this is necessary? expanded_seq_mod.push_back(_ex1); j++; } else { - expanded_seq_mod.push_back(rename_dummy_indices_uniquely(va, expanded_seq[i], true)); + expanded_seq_mod.push_back(skip_idx_rename ? expanded_seq[i] : + rename_dummy_indices_uniquely(va, expanded_seq[i], true)); } } - while (true) { + bool show_must_go_on = true; + while (show_must_go_on) { exvector term = expanded_seq_mod; - for (size_t i=0; i<number_of_adds; i++) { - term[positions_of_adds[i]] = rename_dummy_indices_uniquely(va, expanded_seq[positions_of_adds[i]].op(k[i]), true); - } + for (size_t i=0; i<number_of_adds; i++) + term[positions_of_adds[i]] = skip_idx_rename ? + expanded_seq[positions_of_adds[i]].op(k[i]) : + rename_dummy_indices_uniquely(va, expanded_seq[positions_of_adds[i]].op(k[i]), true); distrseq.push_back((new ncmul(term, true))-> setflag(status_flags::dynallocated | (options == 0 ? status_flags::expanded : 0))); // increment k[] - int l = number_of_adds-1; - while ((l>=0) && ((++k[l]) >= number_of_add_operands[l])) { + size_t l = number_of_adds - 1; + while ((++k[l]) >= number_of_add_operands[l]) { k[l] = 0; - l--; + if (l == 0) { + show_must_go_on = false; + break; + } + else + --l; } - if (l<0) - break; } return (new add(distrseq))-> -- 1.5.3.2
By the way, I failed to apply your patch through the command
patch -p1 < p.diff
Interesting. Both patches work for me (TM), that is, I can apply them to current HEAD (and ginac_1-4) with patch -p1.
What is the correct way to apply it?
patch -p1 < that_file.diff Best regards, Alexei -- All science is either physics or stamp collecting.