symbolic modular integers still broken

[sagetrac-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'

}}}

Depends on #21391

CC: @kcrisman @slel

Component: symbolics

Branch/Commit: u/rws/symbolic_modular_integers_still_broken @ be9ec4e

Reviewer: Ralf Stephan

Issue created by migration from https://trac.sagemath.org/ticket/18787

[sagetrac-tmonteil]

comment:4

Thanks for isolating the problem.

[kcrisman]

comment:5

Thanks very much to both for reporting and simplifying this.

[nbruin]

comment:6

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).

[sagetrac-tmonteil]

comment:7

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]

comment:8

(Or even just disallow it?)

[rwst]

comment:9

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

EDIT: I meant the original case.

[rwst]

comment:10

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.

[rwst]

comment:11

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

[rwst]

Dependencies: pynac-0.3.9.2

[rwst]

comment:14

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 pynac/pynac#77.

[rwst]

Upstream: Reported upstream. Developers acknowledge bug.

[rwst]

Branch: u/rws/bug_with_products_of_symbolic_variables_with_modular_integers

[rwst]

Author: Ralf Stephan

[rwst]

Changed upstream from Reported upstream. Developers acknowledge bug. to none

[rwst]

New commits:

6f9e551

18787: doctests

[rwst]

Commit: 6f9e551

[rwst]

Changed dependencies from pynac-0.3.9.2 to none

[jdemeyer]

comment:17

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
**********************************************************************

[rwst]

Changed branch from u/rws/bug_with_products_of_symbolic_variables_with_modular_integers to none

[rwst]

Changed author from Ralf Stephan to none

[sagetrac-tmonteil]

comment:32

Replying to @rwst:

I believe this is resolved in #21391.

It seems inteed to works better now. But why not adding the tests discussed on the current ticket as you did previously ?

[rwst]

Branch: u/rws/symbolic_modular_integers_still_broken

[rwst]

comment:34

Made dependent on #21391 to prevent merge conflicts.

---

New commits:

7147699	21391: Disallow mixing of pos.char.ring elements and symbolic variables
dd6ad96	Merge branch 'develop' into t/21391/disallow_mixing_of_pos_char_ring_elements_and_symbolic_variables
03ccd07	Merge branch 'develop' into t/21391/disallow_mixing_of_pos_char_ring_elements_and_symbolic_variables
f2933cc	21391: introduce ex.has_finite_parent()
333db3c	Merge branch 'develop' into t/21391/disallow_mixing_of_pos_char_ring_elements_and_symbolic_variables
8700e00	Merge branch 'develop' into t/21391/disallow_mixing_of_pos_char_ring_elements_and_symbolic_variables
c7f8ad2	21391: to prevent fails give the coordinate functions an explicit characteristic
9430605	21391: doctest fixes
be9ec4e	18787: doctest

[rwst]

Commit: be9ec4e

[rwst]

Author: Ralf Stephan

[rwst]

Dependencies: #21391

[koffie]

comment:35

Do the people on this ticket agree that #24072 solves this ticket as is claimed there?

[rwst]

comment:36

Agree.

[tscrim]

Reviewer: Ralf Stephan

[tscrim]

Changed author from Ralf Stephan to none