sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.45k stars 482 forks source link

Behaviour of projective curves over QQbar depend on indeterminate names #32077

Open edd8e884-f507-429a-b577-5d554626c0fe opened 3 years ago

edd8e884-f507-429a-b577-5d554626c0fe commented 3 years ago

As reported on this ask question :

P.<a, y, z> = ProjectiveSpace(QQbar, 2)
f = a^3 + z^3
C = Curve(f)
g = (a + y)^3 + z^3
D = Curve(g)
print(C.intersection_points(D))

leads to

TypeError: Unable to enumerate points over Algebraic Field.

However, if we replace the indeterminate a with x:

P.<x, y, z> = ProjectiveSpace(QQbar, 2)
f = x^3 + z^3
C = Curve(f)
g = (x + y)^3 + z^3
D = Curve(g)
print(C.intersection_points(D))

leads to:

[(-1 : 0 : 1),
 (-1 : 1.500000000000000? - 0.866025403784439?*I : 1),
 (-1 : 1.500000000000000? + 0.866025403784439?*I : 1),
 (0.50000000000000000? - 0.866025403784439?*I : -1.500000000000000? + 0.866025403784439?*I : 1),
 (0.50000000000000000? - 0.866025403784439?*I : 0 : 1),
 (0.50000000000000000? - 0.866025403784439?*I : 0.?e-38 + 1.732050807568878?*I : 1),
 (0.50000000000000000? + 0.866025403784439?*I : -1.500000000000000? - 0.866025403784439?*I : 1),
 (0.50000000000000000? + 0.866025403784439?*I : 0.?e-38 - 1.732050807568878?*I : 1),
 (0.50000000000000000? + 0.866025403784439?*I : 0 : 1)]

CC: @kwankyu @tscrim

Component: algebraic geometry

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

nbruin commented 3 years ago
comment:1

Oddly (don't ask me why I tried), I cannot replicate this behaviour on 8.9.rc0. So it's probably a problem that has been introduced recently, or made apparent by a recent change elsewhere.

slel commented 3 years ago
comment:2

Bisecting further, the example works in Sage 9.0 and fails in Sage 9.1.

edd8e884-f507-429a-b577-5d554626c0fe commented 3 years ago
comment:3

I did a complete bisect, the regression appears in commit 977ace651af9b99689f7b6507f91f8b4e2588ae9 from ticket #27953.

tscrim commented 3 years ago
comment:4

I am very curious how this could happen from the changes in #27953 because it is the factoring that fails with the polynomial

x + 0.500000000000000? + 0.2886751345948129?*I

which is then propagated up. The minimal polynomial of the constant is x^2 - x + 1/3. I can reproduce this directly:

sage: R.<a,y,z> = QQbar[]
sage: x = QQ['x'].gen()
sage: a + QQbar.polynomial_root(x^2 - x + 1/3, CIF(RIF(0.4,0.6), RIF(0.25,0.3)))
a + 0.500000000000000? + 0.2886751345948129?*I
sage: _.factor()  # boom!

Contrast with

sage: R.<x,y,z> = QQbar[]
sage: a = QQ['a'].gen()
sage: x + QQbar.polynomial_root(a^2 - a + 1/3, CIF(RIF(0.4,0.6), RIF(0.25,0.3)))
x + 0.500000000000000? + 0.2886751345948129?*I
sage: _.factor()
x + 0.500000000000000? + 0.2886751345948129?*I

However, the problem is going deeper with quo_rem(self, right) in rings/polynomial/multi_polynomial_element.py. More tracing seems to be required.

kwankyu commented 3 years ago
comment:5

Replying to @tscrim:

I am very curious how this could happen from the changes in #27953 because it is the factoring that fails with the polynomial ...

Before #27953, the ambient projective space of the curve is implicitly constructed without names argument. After #27953, it is always constructed with names set to the variable names of the parent ring of the given multivariate polynomial. Hence now with #27953, the variable names could possibly affect the behavior of methods of curves.

Anyway, the fault is with the method that behaves differently depending on the variable names!

kwankyu commented 3 years ago
comment:6

Perhaps the simplest example:

sage: R.<x,a> = QQbar[]
sage: f = x + QQbar(sqrt(2))
sage: f % x
1.414213562373095?
sage: R.<a,b> = QQbar[]
sage: f = a + QQbar(sqrt(2))
sage: f % a  # boom!

It seems only "a" matters. Strange...

tscrim commented 3 years ago
comment:7

Some more to the mystery:

sage: R.<c,b> = QQbar[]
sage: f = c + QQbar(sqrt(2))
sage: f % c
1.414213562373095?
sage: R.<b,c> = QQbar[]
sage: f = c + QQbar(sqrt(2))
sage: f % c
1.414213562373095?

sage: R.<b,a> = QQbar[]
sage: f = b + QQbar(sqrt(2))
sage: f % b
1.414213562373095?
sage: f = a + QQbar(sqrt(2))
sage: f % a  # Boom

It does seem very specific to calling a variable a:

sage: R.<aa,a> = QQbar[]
sage: f = aa + QQbar(sqrt(2))
sage: f % aa
1.414213562373095?
nbruin commented 3 years ago
comment:8

A tiny sliver of information. For:

sage: sage: R.<b,a> = QQbar[]
sage: sage: f = a + QQbar(sqrt(2))
sage: f % a

I'm getting

TypeError: Singular error:
   ? division(`number`,`number`) failed
   ? error occurred in or before STDIN line 26: `def sage10=division(sage8,sage9);`

Indeed, in the debugger (one level up from the raise error) I'm ending up in the singular interface (Note it's the singular interface; not libsingular! That's already fishy), trying to execute exactly that statement:

ipdb> p self.eval('sage8')
'(2*a)'
ipdb> p self.eval('sage9')
'(a)'
ipdb> p self.eval('basering')
'// coefficients: QQ[a]/(a^2-2)
 // number of vars : 2
 //        block   1 : ordering dp
 //                  : names    b @@(1)
 //        block   2 : ordering C'

Note that the 'a' here is already used for sqrt(2), so I can imagine this produces a clash.

EDIT: the method as_number_field_element seems to have a preference to using a as a variable name. Of course, this causes problems if we then use a strings-based interface combined with a polynomial ring over it that also uses a as a variable. Would the situation improve if we were to use libsingular instead?

Of course, it's still strange that the division doesn't work: we're not particularly dividing by 0, even with the name collision (I guess we are dividing constants as far as singular is concerned. Perhaps it doesn't like that?)

kwankyu commented 3 years ago
comment:9

Replying to @nbruin:

EDIT: the method as_number_field_element seems to have a preference to using a as a variable name. Of course, this causes problems if we then use a strings-based interface combined with a polynomial ring over it that also uses a as a variable. Would the situation improve if we were to use libsingular instead?

So here Sage uses Sage variable names as corresponding Singular variable names. It is surprising to me that there is no translation mechanism working between Sage variable names and internal Singular variable names.

Of course, it's still strange that the division doesn't work: we're not particularly dividing by 0, even with the name collision (I guess we are dividing constants as far as singular is concerned. Perhaps it doesn't like that?)

division performs quotient and remainder division. It is inappropriate to apply it to elements of a field (QQ[a]/(a^2-2)). Hence the clash.

kwankyu commented 3 years ago
comment:10

Replying to @kwankyu:

So here Sage uses Sage variable names as corresponding Singular variable names. It is surprising to me that there is no translation mechanism working between Sage variable names and internal Singular variable names.

This turns out a general problem of the Singular interface.

sage: F.<a> = GF(2^3)
sage: R.<x,y> = F[]
sage: f = x+a
sage: f
x + (a)
sage: f._singular_()
x+(a)
sage: R.<a,b> = F[]
sage: f = a + F.gen()
sage: f
a + (a)
sage: f._singular_()
0 
nbruin commented 3 years ago
comment:11

Replying to @kwankyu:

So here Sage uses Sage variable names as corresponding Singular variable names. It is surprising to me that there is no translation mechanism working between Sage variable names and internal Singular variable names.

Surprising, perhaps, but the standard behaviour of interfaces. Also, in this case not the root cause of the problem. By the time the expression arrives at the singular interface, all the names are "sage generator names": the expression is an element of NumberField(x^2-2,names=['a'])['a','b']. Unless we're going to include type information in the identifier translation to Singular, we're always going to have a clash.

A work-around for this particular example is to equip qqbar.py:2665 with a way to select a less obvious name. I'm afraid people would get really unhappy if there were no choice in that name or even if by default it wouldn't be something nice and short such as 'a'. I expect that this routine (through as_number_field_element) gets used quite a bit interactively.

kwankyu commented 3 years ago
comment:12

Replying to @nbruin:

Surprising, perhaps, but the standard behaviour of interfaces.

The standard behavior luckily didn't cause a trouble until the current case. But it seems it is inherently flawed. A simple-minded approach to fix the problem is to always assign unique Singular identifier name for every generator names of a new Sage ring. We don't need to work around. Would this be difficult to implement for interpreter interfaces? I have little understanding of the details of interpreter interfaces.

nbruin commented 3 years ago
comment:13

Replying to @kwankyu:

Replying to @nbruin:

Surprising, perhaps, but the standard behaviour of interfaces.

The standard behavior luckily didn't cause a trouble until the current case. But it seems it is inherently flawed. A simple-minded approach to fix the problem is to always assign unique Singular identifier name for every generator names of a new Sage ring. We don't need to work around. Would this be difficult to implement for interpreter interfaces? I have little understanding of the details of interpreter interfaces.

The big problem for this is the dictionary that is necessary for the translation back. The dictionary must live in a place accessible to the interface that is asked to translate one of its objects into a sage object. The most straightforward version would key on the interface name-string and have as value the object. Big problem: now we have a global reference, and therefore a memory leak (assuming the interface is long-lived). We could make the dictionary weak-valued. That does not eliminate all memory leak possibilities, but gets a little closer. However, now you can hold on to an interface object that refers to objects that were coming from sage, but are no longer present there. Should such "references across systems" keep each other alive? For SR to maxima this was answered affirmative and as a result, SR symbol creation should be assumed leaky (that's already true for pynac). I suspect it will be more problematic for interfaces like singular, which could for instance interface for many curves over finite fields.

The original design was intentionally naive, providing just basic interfacing. I think the basic interface level doesn't have enough information available to address lifetime management problems.

For this particular example, I think the right fix is actually to see if libsingular can be used instead. This is pretty basic arithmetic, so an expect interface is a pretty high-overhead solution.

tscrim commented 3 years ago
comment:14

Further example along the lines of comment:10:

sage: F.<a> = QQ.extension(SR.var('y')^2 - 2)
sage: R.<a,y> = F[]
sage: f = a + F.gen()
sage: f._singular_()
(2*a)
tscrim commented 3 years ago
comment:15

We can see a manifestation of the name clash here:

sage: F.<a> = QQ.extension(SR.var('y')^2 - 2)
sage: R.<x,y> = F[]
sage: R._singular_()
polynomial ring, over a field, global ordering
// coefficients: QQ[a]/(a^2-2)
// number of vars : 2
//        block   1 : ordering dp
//                  : names    x y
//        block   2 : ordering C
sage: R.<a,y> = F[]
sage: R._singular_()
polynomial ring, over a field, global ordering
// coefficients: QQ[a]/(a^2-2)
// number of vars : 2
//        block   1 : ordering dp
//                  : names    @@(1) y
//        block   2 : ordering C

One other way to get around this would be for the @handle_AA_and_QQbar to redirect to the corresponding method for the constructed number field element should it exist. However, I think this would be make calling the method really slow on these elements. Really we should have a special class for polynomials over AA/QQbar rather than use this decorator. This will fix the original issue with the ticket and likely improve the speed for these 4 methods.

Of course, we should still make some reasonable attempt at solving the issue with the interface. Although I could see that as a separate issue.

kwankyu commented 3 years ago
comment:16

Replying to @tscrim:

One other way to get around this would be for the @handle_AA_and_QQbar to redirect to the corresponding method for the constructed number field element should it exist. However, I think this would be make calling the method really slow on these elements. Really we should have a special class for polynomials over AA/QQbar rather than use this decorator. This will fix the original issue with the ticket and likely improve the speed for these 4 methods.

Related: #25351 where quo_rem got @handle_AA_and_QQbar decorator.