[GiNaC-devel] (attempt 2) fix ncmul::expand regression

[Alexei Sheplyakov]

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.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 827 bytes
Desc: Digital signature
URL: <http://www.ginac.de/pipermail/ginac-devel/attachments/20071105/b9437c65/attachment.sig>