symbolic modular integers still broken

[tmonteil]

sage: f(x) = Zmod(7)(1) * x**2 + Zmod(9)(1) * x**3
sage: f(1)
2
sage: Zmod(7)(1) + Zmod(9)(1)

... TypeError?: unsupported operand parent(s) for '+': 'Ring of integers modulo 7' and 'Ring of integers modulo 9'

}}}

[tmonteil]

Thanks for isolating the problem.

[kcrisman]

Thanks very much to both for reporting and simplifying this.

[nbruin]

A different issue, but it might be related:

sage: F=sum((i+1)*x^i for i in [0..20])
sage: G=sum(Zmod(7)(i+1)*x^i for i in [0..20])
sage: F*Zmod(7)(1) - G
x^19 + 2*x^18 + 0*x^13 + 0*x^6
sage: G*Zmod(7)(1) - G
0

so there seems to be something fishy with symbolic multiplication involving integers and elements of Z/n in general. It's not just a zero-divisor problem.

It's not just powers either:

sage: V=[SR.var("x%s"%i) for i in [0..20]]
sage: F=sum((i+1)*V[i] for i in [0..20])
sage: G=sum(Zmod(7)(i+1)*V[i] for i in [0..20])
sage: sage: F*Zmod(7)(1)-G
-x0 + 0*x13 + 6*x14 + 0*x20 + 3*x3 + 0*x6
sage: F-G 
14*x13 + 21*x20 + 7*x6

The last one is correct, since the terms with x6, x20 and x13 are really missing from G. This illustrates why mixing characteristics in SR is always going to be a mess (even if non-zero characteristic works properly otherwise).

[tmonteil]

It seems to be a pynac/ginac issue, right ? So what should we do on our side ? Add a stopgap during conversion/coercion Zmod(n) -> SR ?

[kcrisman]

(Or even just disallow it?)

[rws]

This is working in pynac-0.4.1 (but not 0.3.9.1).

EDIT: I meant the original case.

[rws]

Replying to nbruin:

It's not just powers either:

sage: V=[SR.var("x%s"%i) for i in [0..20]]
sage: F=sum((i+1)*V[i] for i in [0..20])
sage: G=sum(Zmod(7)(i+1)*V[i] for i in [0..20])
sage: sage: F*Zmod(7)(1)-G
-x0 + 0*x13 + 6*x14 + 0*x20 + 3*x3 + 0*x6
sage: F-G 
14*x13 + 21*x20 + 7*x6

This is different in 0.4.1:

sage: sage: sage: F*Zmod(7)(1)-G
-x0 + 0*x13 + 0*x20 + 4*x9
sage: F-G
14*x13 + 21*x20 + 7*x6
sage: F
x0 + 2*x1 + 11*x10 + 12*x11 + 13*x12 + 14*x13 + 15*x14 + 16*x15 + 17*x16 + 18*x17 + 19*x18 + 20*x19 + 3*x2 + 21*x20 + 4*x3 + 5*x4 + 6*x5 + 7*x6 + 8*x7 + 9*x8 + 10*x9
sage: G
x0 + 2*x1 + 4*x10 + 5*x11 + 6*x12 + x14 + 2*x15 + 3*x16 + 4*x17 + 5*x18 + 6*x19 + 3*x2 + 4*x3 + 5*x4 + 6*x5 + x7 + 2*x8 + 3*x9

EDIT: Added F,G.

[rws]

Correction: this is not pynac-0.4.1 but pynac master. Narrowing down the reponsible change...

[rws]

The following patch works for these two examples, just to point at the direction further completion is needed:

diff --git a/ginac/expairseq.cpp b/ginac/expairseq.cpp
index 2abb7b7..acef09b 100644
--- a/ginac/expairseq.cpp
+++ b/ginac/expairseq.cpp
@@ -1207,16 +1207,17 @@ void expairseq::combine_same_terms_sorted_seq()
 	// must_copy will be set to true the first time some combination is 
 	// possible from then on the sequence has changed and must be compacted
 	bool must_copy = false;
+        numeric itin1_c = ex_to<numeric>(itin1->coeff);
 	while (itin2!=last) {
 		if (itin1->rest.compare(itin2->rest)==0 &&
 				unlikely(!is_a<infinity>(itin1->rest))) {
-			itin1->coeff = ex_to<numeric>(itin1->coeff).
+			itin1->coeff = itin1_c.
 			               add_dyn(ex_to<numeric>(itin2->coeff));
 			if (expair_needs_further_processing(itin1))
 				needs_further_processing = true;
 			must_copy = true;
 		} else {
-			if (!ex_to<numeric>(itin1->coeff).is_zero()) {
+			if (not itin1_c.is_zero() or itin1_c.is_parent_pos_char()) {
 				if (must_copy)
 					*itout = *itin1;
 				++itout;
@@ -1225,7 +1226,7 @@ void expairseq::combine_same_terms_sorted_seq()
 		}
 		++itin2;
 	}
-	if (!ex_to<numeric>(itin1->coeff).is_zero()) {
+	if (not itin1_c.is_zero() or itin1_c.is_parent_pos_char()) {
 		if (must_copy)
 			*itout = *itin1;
 		++itout;

The relevant ticket would be ​https://github.com/pynac/pynac/issues/77.

[rws]

New commits:

​6f9e551

18787: doctests

[jdemeyer]

With Sage-6.8.rc0:

**********************************************************************
File "src/sage/symbolic/expression.pyx", line 2821, in sage.symbolic.expression.Expression._add_
Failed example:
    (A + 3*B)*Zmod(9)(6)
Expected:
    6*A
Got:
    0* + 6*A
**********************************************************************
File "src/sage/symbolic/expression.pyx", line 2823, in sage.symbolic.expression.Expression._add_
Failed example:
    (3*A + 3*B)*Zmod(9)(6)
Expected:
    0
Got:
    0*
**********************************************************************
File "src/sage/symbolic/expression.pyx", line 2825, in sage.symbolic.expression.Expression._add_
Failed example:
    (3*A + 3*B)*Zmod(9)(6)*A
Exception raised:
    Traceback (most recent call last):
      File "/usr/local/src/sage-git/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 496, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/usr/local/src/sage-git/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 858, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.symbolic.expression.Expression._add_[24]>", line 1, in <module>
        (Integer(3)*A + Integer(3)*B)*Zmod(Integer(9))(Integer(6))*A
      File "sage/structure/element.pyx", line 1849, in sage.structure.element.RingElement.__mul__ (build/cythonized/sage/structure/element.c:16581)
        return (<RingElement>left)._mul_(<RingElement>right)
      File "sage/symbolic/expression.pyx", line 3148, in sage.symbolic.expression.Expression._mul_ (build/cythonized/sage/symbolic/expression.cpp:19837)
        x = gmul(left._gobj, _right._gobj)
    OverflowError: numeric::div(): division by zero
**********************************************************************

[jdemeyer]

Also a detail: it's better style to use

sage: x = foo

instead of

sage: x=foo

[rws]

Seems the backport from Pynac-0.4.2 missed something. When will C++11 be mandatory in Sage?

[jdemeyer]

Replying to rws:

When will C++11 be mandatory in Sage?

I think I already mentioned this: I recommend you write to sage-devel with this request.

[jdemeyer]

Let me add that the hard part will be to define the precise subset of C++11 that you need.

It's easy to require that the C++ compiler supports the -std=c++0x or -std=c++11 option, but I don't know if that is sufficient.

[rws]

Replying to jdemeyer:

Let me add that the hard part will be to define the precise subset of C++11 that you need.

Why? We have already committed to gcc-4.6 in our conversation. For the other system without gcc there will have to be an Xcode version that has equivalent functionality. These two versions will be made the minimum. No other definition necessary.

[jdemeyer]

Replying to rws:

We have already committed to gcc-4.6 in our conversation

Really? I understood that you were not sure if GCC 4.6 was in fact sufficient. But if you are, that's fine for me.

Note that, for various reasons, we actually do not support GCC 4.6 in Sage, which would make GCC 4.7 the minimum.

If you're willing to go ahead with this, you should:

1. write to sage-devel explaining why you want C++11 support and that this means requiring GCC 4.7.
2. create a ticket for requiring C++11 and GCC 4.7 in Sage.
3. create a ticket for upgrading Pynac to the version and make it depend on the ticket in 2.

[rws]

Replying to jdemeyer:

With Sage-6.8.rc0:

Got:
    0* + 6*A
Got:
    0*

Found it. The backport was indeed incomplete. Fix in pynac-0.3.9.3.

[vdelecroix]

Why is it in sage-pending?

What about the following (inspired from comment:6

sage: f(x) = Zmod(7)(3) * x**2 + Zmod(9)(3) * x**3
sage: f(1)
** BOOM **

while

sage: f(x) = Zmod(7)(1) * x**2 + Zmod(9)(1) * x**3
sage: f(1)
2

Vincent

[rws]

Replying to vdelecroix:

Why is it in sage-pending?

I didn't want patchbot on it.