Move invariant_generators to libsingular

[miguelmarco]

This patch moves the .invariant_generators() method of finite matrix groups to libsingular, which simplifies much the code.

It also adds the possibility of defining the ring in which the result should be:

sage: m1 = matrix(QQ, [[0, -1], [1, 0]])
sage: G = MatrixGroup([m1])
sage: G.invariant_generators()
[x0^2 + x1^2, x0^4 + x1^4, x0^3*x1 - x0*x1^3]
sage: R.<x,y> = QQ[]
sage: G.invariant_generators(R)
[x^2 + y^2, x^4 + y^4, x^3*y - x*y^3]

CC: @simon-king-jena @wdjoyner

Component: group theory

Author: Miguel Marco

Branch/Commit: public/ticket/19391 @ ea82aa9

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

[miguelmarco]

Branch: u/mmarco/invariants

[simon-king-jena]

comment:3

The code now is simpler. But there was a reason to have it like that: Singular isn't good at listing the elements of a group (which is needed for the Reynolds operator).

If I recall correctly, there were examples for which the computation of the Reynolds operator in Singular was too slow. Apparently these examples didn't go into the doctests. But perhaps they are available at the trac ticket for the original version of the code?

---

New commits:

24817f2

Moved invariant_generators to libsingular, added option to fix the ring

[simon-king-jena]

Commit: 24817f2

[miguelmarco]

comment:4

Ok, i read in the header that you moved the computation of the reynolds operator to gap, but i didn't see any call to gap in the code. Now i see it, you mean this part, right?:

ReyName = 't'+singular._next_var_name()
singular.eval('matrix %s[%d][%d]'%(ReyName,self.cardinality(),n))
for i in range(1,self.cardinality()+1):
     M = Matrix(elements[i-1],F)
     D = [{} for foobar in range(self.degree())]
     for x,y in M.dict().items():
     D[x[0]][x[1]] = y
      for row in range(self.degree()):
          for t in D[row].items():
          singular.eval('%s[%d,%d]=%s[%d,%d]+(%s)*var(%d)'
          %(ReyName,i,row+1,ReyName,i,row+1, repr(t[1]),t[0]+1))

What i don't understand is this part:

else:
    ReyName = 't'+singular._next_var_name()
    singular.eval('list %s=group_reynolds((%s))'%(ReyName,Lgens))
    IRName = 't'+singular._next_var_name()
    singular.eval('matrix %s = invariant_algebra_reynolds(%s[1])'%(IRName,ReyName))

If i am getting it right, it is supposed to cover the case where there are no elements in the group. In that case we should just return the ring itself.

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Branch pushed to git repo; I updated commit sha1. New commits:

6a025ba

Compute reynolds operator before passing it to singular

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from 24817f2 to 6a025ba

[miguelmarco]

comment:6

Ok, now it computes the reynolds operator before passing it to singular.

I am now working on the modular case. I having trouble getting the output of invariant_ring from libsingular. The singular command is supposed to return three matrices, but calling it through libsingular only gets the first one:

sage: from sage.libs.singular.function import singular_function
sage: import sage.libs.singular.function_factory
sage: sage.libs.singular.function_factory.lib('finvar.lib')
sage: inring = singular_function('invariant_ring')
sage: F=FiniteField(2)
sage: R.<x,y> = F[]
sage: m1 = matrix(R, 2, [0,1,1,0])
sage: inring(m1)
[x + y   x*y]

Any clue about how to get around this?

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Branch pushed to git repo; I updated commit sha1. New commits:

2fa234a

Modular case

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from 6a025ba to 2fa234a

[miguelmarco]

comment:8

Ok, i think it should be ok now. I added the modular case.

I think that this code should be faster than the previous one, since it does the same, but using the faster libsingular interface rather than the string based one. If you have some interesting examples to test, please benchmark them.

[cheuberg]

comment:9

merge conflict.

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Branch pushed to git repo; I updated commit sha1. New commits:

90b47ce

Merge branch 'develop' into t/19391/invariants

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from 2fa234a to 90b47ce

[fchapoton]

comment:13

one failing doctest (shift in the indices)

[fchapoton]

comment:14

I fixed the failing doctest

---

New commits:

2558eaa	Merge branch 'u/mmarco/invariants' in 9.6.rc4
31d3c84	fix doctests

[fchapoton]

Changed commit from 90b47ce to 31d3c84

[fchapoton]

Changed branch from u/mmarco/invariants to public/ticket/19391

[fchapoton]

comment:15

is there anybody around still interested in this ticket ? It now passes the doctests. It may deserve some benchmarking.

[miguelmarco]

comment:16

For my part, i would set it to positive review if you agree.

[fchapoton]

comment:17

I would like to have at least one benchmark here (before / after), please.

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Branch pushed to git repo; I updated commit sha1. New commits:

ea82aa9

Force base ring to be a field, to prevent singular segfault

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from 31d3c84 to ea82aa9

[miguelmarco]

comment:19

Surprisingly, I have found that the new code is actually a few miliseconds slower that the older one.

While at profiling, I also caught a possible source of segfaults.

---

New commits:

ea82aa9

Force base ring to be a field, to prevent singular segfault

[fchapoton]

comment:20

red branch => needs_work