more libgap in permutation groups ; get rid of _libgap_init_ #36889

fchapoton · 2023-12-15T12:39:35Z

Moving forward from gap pexpect interface to libgap interface :

using more libgap in the permutation groups code
get rid of the barely used _libgap_init_ method in favor of the unique _gap_init_ method.

📝 Checklist

The title is concise, informative, and self-explanatory.
The description explains in detail what this PR is about.
I have created tests covering the changes.

dimpase · 2023-12-16T19:48:12Z

src/sage/groups/class_function.py

@@ -123,20 +125,20 @@ def __init__(self, G, values):
        if isinstance(values, GapElement) and gap.IsClassFunction(values):
            self._gap_classfunction = values
        else:
-            self._gap_classfunction = gap.ClassFunction(G, list(values))
+            self._gap_classfunction = gap.ClassFunction(G, gap(values))


What is the point of calling gap() here? This is a call to pexpect GAP, what were getting rid of, right?

Well, this is inside Classfunction_gap. I have made so that Classfunction now calls ClassFunction_libgap by default, unless the second argument is a GapPexpect element.

Of course, it would be better to get rid completely of Classfunction_gap but maybe in another ticket ?

Do you have any idea about the failing doctests in class_function.py ?

Do you have any idea about the failing doctests in class_function.py ?

it comes from the fact that GAP operates on characters associated to a character table,
not to a group, and is unable to identify these tables for $\chi_1$ and $\chi_2$ in the 1st failing test.
Not clear how to fix this easily.

sage: chi1 = ClassFunction(G, [3, 1, -1, 0, -1]) sage: isinstance(chi1, ClassFunction_libgap) True sage: chi2 = ClassFunction(G, [1, -1, 1, 1, -1]) sage: isinstance(chi2, ClassFunction_libgap) True sage: chi1._gap_classfunction ClassFunction( CharacterTable( Sym( [ 1 .. 4 ] ) ), [ 3, 1, -1, 0, -1 ] ) sage: chi2._gap_classfunction ClassFunction( CharacterTable( Sym( [ 1 .. 4 ] ) ), [ 1, -1, 1, 1, -1 ] ) sage: chi1._gap_classfunction*chi2._gap_classfunction --------------------------------------------------------------------------- GAPError Traceback (most recent call last) Cell In[11], line 1 ----> 1 chi1._gap_classfunction*chi2._gap_classfunction File /mnt/opt/Sage/sage-clang/src/sage/structure/element.pyx:1513, in sage.structure.element.Element.__mul__() 1511 cdef int cl = classify_elements(left, right) 1512 if HAVE_SAME_PARENT(cl): -> 1513 return (<Element>left)._mul_(right) 1514 if BOTH_ARE_ELEMENT(cl): 1515 return coercion_model.bin_op(left, right, mul) File /mnt/opt/Sage/sage-clang/src/sage/libs/gap/element.pyx:1056, in sage.libs.gap.element.GapElement._mul_() 1054 try: 1055 sig_GAP_Enter() -> 1056 sig_on() 1057 result = GAP_PROD(self.value, (<GapElement>right).value) 1058 sig_off() GAPError: Error, no product of class functions of different tables

dimpase · 2023-12-17T12:33:09Z

there also seems to be a disconnect between Sage's conjugacy classes of $S_n$ and underlying (lib)GAP.
One has to tell the underlying GAP that these is the classes to use, otherwise it seems it's just by luck no discrepancies come in here.
E.g. in this failing test for character multiplication it seems it's just a coincidence that GAP uses the same ordering of conjugacy classes as in G.conjugacy_classes().

fchapoton · 2023-12-21T10:41:35Z

the failure can be traced to the fact that the underlying character tables are not identical:

sage: ct1 = chi1._libgap_().UnderlyingCharacterTable()
sage: ct2 = chi2._libgap_().UnderlyingCharacterTable()
sage: ct1.IsIdenticalObj(ct2)
false

although they share the same group but not quite :

sage: g1=ct1.UnderlyingGroup()
sage: g2=ct2.UnderlyingGroup()
sage: g1.IsIdenticalObj(g2)
false

see https://github.com/gap-system/gap/blob/master/lib/ctblfuns.gi

fchapoton · 2023-12-21T10:57:30Z

and one can find that

sage: S = SymmetricGroup(4)
sage: g1 = S._libgap_()
sage: g2 = S._libgap_()
sage: g1.IsIdenticalObj(g2)
false

dimpase · 2023-12-21T15:51:20Z

this is very strange. I would expect the 1st call to _libgap_() to create a record in S, and subsequent calls to reuse it

fchapoton · 2023-12-21T19:32:26Z

the good old expect interface behaves better :

sage: S = SymmetricGroup(4)
sage: g1 = S._gap_()
sage: g2 = S._gap_()
sage: g1.IsIdenticalObj(g2)
true

dimpase · 2023-12-21T21:14:34Z

I don't know permgroup.py code.
How about

--- a/src/sage/groups/perm_gps/permgroup.py
+++ b/src/sage/groups/perm_gps/permgroup.py
@@ -684,7 +684,8 @@ class PermutationGroup_generic(FiniteGroup):
         """
         if self._libgap is not None:
             return self._libgap
-        return super()._libgap_()
+        self._libgap = super()._libgap_()
+        return self._libgap
 
     # Override the default _libgap_ to use the caching as self._libgap
     _libgap_ = gap

This fixes this weird false.

dimpase · 2023-12-21T21:28:09Z

now all tests in class_function.py pass. Some noise in doctests of permutation_group.py (same groups, different sets of generators, will fix them now)

dimpase · 2023-12-21T22:19:22Z

OK, I'm not 100% sure about the fix

             sage: G = PermutationGroup([[(1,2,3,4)],[(1,2)]])
             sage: G.strong_generating_system()
             [[(), (1,2)(3,4), (1,3)(2,4), (1,4)(2,3)],
-             [(), (2,4), (2,3,4)], [(), (3,4)], [()]]
+            [(), (2,4,3), (2,3,4)],
+            [(), (3,4)],
+            [()]]

originally one had the sequence of subgroups $A_4$, $S_3$, $S_2$, and now it's
$A_4$, $A_3$, $S_2$. Note that in both cases it's not a sequence of sugroups, each subgroup of the previous - and this contracts to what I know about such systems.
(cf https://en.wikipedia.org/wiki/Strong_generating_set too)

It seems it's a bug, which now manifests itself differently. Something funny is going on, but I don't think it's due to my change.

EDIT
It's actually OK, I think, cause each list $\ell_j$ of permutations shoud be added to
$\ell_{j+1}, \ell_{j+2},\dots$ to generate $H_j$.
So in both cases one has $H_4=id$, $H_3=S_2$, $H_2=S_3$, $H_1=S_4$.

fchapoton · 2023-12-22T08:41:02Z

still one broken doctest in src/sage/algebras/fusion_rings/fusion_double.py that needs to be investigated

fchapoton · 2023-12-22T13:09:15Z

The issue in fusion rings may be caused by the fact that the trivial character no longer comes first in the list :

sage: G = groups.permutation.Cyclic(2)
sage: [list(t) for t in G.irreducible_characters()]
[[1, -1], [1, 1]]

but I am not sure.

fchapoton · 2023-12-22T13:11:23Z

there is indeed a difference of behaviour :


sage: [list(ClassFunction(G,irr)) for irr in G._gap_().Irr()]
[[1, 1], [1, -1]]
sage: [list(ClassFunction(G,irr)) for irr in G._libgap_().Irr()]
[[1, -1], [1, 1]]

dimpase · 2023-12-22T14:43:56Z

I can try to look at it tonight.

dimpase · 2023-12-22T22:52:19Z

Yes, that's in line 426 of src/sage/algebras/fusion_rings/fusion_double.py.

I don't see what they do there, let's ask @dwbump @tscrim

Do you assume any fixed order of the irreducible characters in the character tables?
(e.g. that the trivial character is always 1st ?)

dimpase · 2023-12-22T23:26:50Z

I tried modifying irreducible_characters() and character_table() in permgroup.py so that the trivial character always comes 1st, and it allows this doctest to pass. So, indeed, this appears to be the root cause here.

dimpase · 2023-12-22T23:37:00Z

However, doing such a change in SageMath is bound to cause a discrepancy, as we really don't want G.character_table() and G._libgap_().Irr() to give different orders.

fchapoton · 2023-12-23T14:08:07Z

The method _one_basis_ is assuming that 0 is the index of the unit in the fusion ring

tscrim · 2023-12-24T11:32:35Z

I don’t think there is anything else besides the one_basis method that uses that the trivial character is the first one. So it is easy enough to fix the issue by just searching in the list of characters for it.

dimpase · 2023-12-24T12:18:46Z

OK, thanks.

fchapoton · 2023-12-25T18:04:05Z

here is a tentative

dimpase · 2023-12-27T13:40:05Z

as a side job, can we unify irreducible_characters and character_table for permutation groups? Both use libgap's Irr(), probably the latter should use the former instead.

This fixes weird sage: S = SymmetricGroup(4) sage: g1 = S._libgap_() sage: g2 = S._libgap_() sage: g1.IsIdenticalObj(g2) false (one gets true now)

dimpase · 2023-12-28T00:59:08Z

Let us do the following change:

--- a/src/sage/structure/sage_object.pyx
+++ b/src/sage/structure/sage_object.pyx
@@ -755,7 +755,7 @@ cdef class SageObject:
 
     def _libgap_(self):
         from sage.libs.gap.libgap import libgap
-        return libgap.eval(self._gap_init_())
+        return libgap.eval(self)
 
     def _gp_(self, G=None):
         if G is None:

Your libgap.eval(self._gap_init_()) creates a pexpect GAP handle for every libgap object used in Sage, and then passes it in text form to libgap. This is very inefficient.

github-actions · 2023-12-28T09:47:40Z

Documentation preview for this PR (built with commit b20ef4f; changes) is ready! 🎉

dimpase

looks good to me

…gap_init_ Moving forward from gap pexpect interface to libgap interface : - using more libgap in the permutation groups code - get rid of the barely used `_libgap_init_` method in favor of the unique `_gap_init_` method. ### 📝 Checklist - [x] The title is concise, informative, and self-explanatory. - [x] The description explains in detail what this PR is about. - [x] I have created tests covering the changes. URL: sagemath#36889 Reported by: Frédéric Chapoton Reviewer(s): Dima Pasechnik, Frédéric Chapoton

fchapoton added the s: needs work label Dec 16, 2023

dimpase reviewed Dec 16, 2023

View reviewed changes

fchapoton force-pushed the get_rid_libgap_init branch from 1170be0 to fa84162 Compare December 21, 2023 10:22

fchapoton added s: needs review c: interfaces and removed s: needs work labels Dec 26, 2023

fchapoton and others added 4 commits December 27, 2023 13:40

more libgap in permutation groups ; get rid of _libgap_init_

ef3a6e9

switch class functions to libgap by default

879963c

some fixes

5dfc256

cache the result of _libgap_() in PermutationGroup

1310d8a

This fixes weird sage: S = SymmetricGroup(4) sage: g1 = S._libgap_() sage: g2 = S._libgap_() sage: g1.IsIdenticalObj(g2) false (one gets true now)

dimpase and others added 2 commits December 27, 2023 13:40

fix doctests in permutation_group.py

7e416a9

looking for the unit character of the full group.

fc2d24d

dimpase force-pushed the get_rid_libgap_init branch from f1797e5 to fc2d24d Compare December 27, 2023 13:40

do not call pexpect GAP from _libgap_()

b20ef4f

dimpase added s: positive review and removed s: needs review labels Dec 28, 2023

dimpase approved these changes Dec 28, 2023

View reviewed changes

vbraun merged commit b3d1c67 into sagemath:develop Jan 14, 2024
19 of 20 checks passed

mkoeppe added this to the sage-10.3 milestone Jan 14, 2024

github-actions bot removed the s: positive review label Jan 14, 2024

tscrim mentioned this pull request Jan 15, 2024

Add minimal_normal_subgroups and maximal_normal_subgroups functions #37038

Merged

4 tasks

fchapoton deleted the get_rid_libgap_init branch January 15, 2024 07:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

more libgap in permutation groups ; get rid of _libgap_init_ #36889

more libgap in permutation groups ; get rid of _libgap_init_ #36889

fchapoton commented Dec 15, 2023

dimpase Dec 16, 2023

fchapoton Dec 17, 2023 •

edited

dimpase Dec 17, 2023

dimpase commented Dec 17, 2023

fchapoton commented Dec 21, 2023 •

edited

fchapoton commented Dec 21, 2023 •

edited

dimpase commented Dec 21, 2023 •

edited

fchapoton commented Dec 21, 2023

dimpase commented Dec 21, 2023

dimpase commented Dec 21, 2023

dimpase commented Dec 21, 2023 •

edited

fchapoton commented Dec 22, 2023

fchapoton commented Dec 22, 2023

fchapoton commented Dec 22, 2023

dimpase commented Dec 22, 2023

dimpase commented Dec 22, 2023

dimpase commented Dec 22, 2023 •

edited

dimpase commented Dec 22, 2023

fchapoton commented Dec 23, 2023 •

edited

tscrim commented Dec 24, 2023

dimpase commented Dec 24, 2023

fchapoton commented Dec 25, 2023

dimpase commented Dec 27, 2023

dimpase commented Dec 28, 2023

github-actions bot commented Dec 28, 2023

dimpase left a comment

more libgap in permutation groups ; get rid of _libgap_init_ #36889

more libgap in permutation groups ; get rid of _libgap_init_ #36889

Conversation

fchapoton commented Dec 15, 2023

📝 Checklist

dimpase Dec 16, 2023

Choose a reason for hiding this comment

fchapoton Dec 17, 2023 • edited

Choose a reason for hiding this comment

dimpase Dec 17, 2023

Choose a reason for hiding this comment

dimpase commented Dec 17, 2023

fchapoton commented Dec 21, 2023 • edited

fchapoton commented Dec 21, 2023 • edited

dimpase commented Dec 21, 2023 • edited

fchapoton commented Dec 21, 2023

dimpase commented Dec 21, 2023

dimpase commented Dec 21, 2023

dimpase commented Dec 21, 2023 • edited

fchapoton commented Dec 22, 2023

fchapoton commented Dec 22, 2023

fchapoton commented Dec 22, 2023

dimpase commented Dec 22, 2023

dimpase commented Dec 22, 2023

dimpase commented Dec 22, 2023 • edited

dimpase commented Dec 22, 2023

fchapoton commented Dec 23, 2023 • edited

tscrim commented Dec 24, 2023

dimpase commented Dec 24, 2023

fchapoton commented Dec 25, 2023

dimpase commented Dec 27, 2023

dimpase commented Dec 28, 2023

github-actions bot commented Dec 28, 2023

dimpase left a comment

Choose a reason for hiding this comment

fchapoton Dec 17, 2023 •

edited

fchapoton commented Dec 21, 2023 •

edited

fchapoton commented Dec 21, 2023 •

edited

dimpase commented Dec 21, 2023 •

edited

dimpase commented Dec 21, 2023 •

edited

dimpase commented Dec 22, 2023 •

edited

fchapoton commented Dec 23, 2023 •

edited