Skip to content

do not cache gap twice #41562

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
mantepse wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

do not cache gap twice #41562

mantepse wants to merge 1 commit into from
+9 −10

Conversation

@mantepse
Copy link
Contributor

@mantepse mantepse commented Jan 30, 2026

Currently, in permgroup.py, we are apparently caching the gap function twice: once using the decorator, and once by putting the result into an attribute.

Apparently, removing the decorator is a performance improvement:

sage: from sage.rings.species import MolecularSpecies, PolynomialSpecies
sage: M = MolecularSpecies("X"); P = PolynomialSpecies(QQ, "X")
sage: n=5; l = [P(m.permutation_group()[0]) for m in M.subset(n)]

sage: %timeit [p.hadamard_product(q) for p in l for q in l]

gives me

6.98 s ± 189 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)

with the decorator and

sage: %timeit [p.hadamard_product(q) for p in l for q in l]
6.35 s ± 198 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)

without.

What I find very surprising is that the following is much slower. I would really like to know why.

diff --git a/src/sage/groups/perm_gps/permgroup.py b/src/sage/groups/perm_gps/permgroup.py
index cf8700476f8..df224194d2c 100644
--- a/src/sage/groups/perm_gps/permgroup.py
+++ b/src/sage/groups/perm_gps/permgroup.py
@@ -697,7 +697,8 @@ class PermutationGroup_generic(FiniteGroup):
         if self._libgap is None:
             # why is self._libgap = libgap.Group(self._gens) and
             # adding methods to permgroup_named slower?
-            self._libgap = super()._libgap_()
+            # self._libgap = super()._libgap_()
+            self._libgap = libgap.Group(self._gens)
         return self._libgap
 
     # Override the default _libgap_ to use the caching as self._libgap
diff --git a/src/sage/groups/perm_gps/permgroup_named.py b/src/sage/groups/perm_gps/permgroup_named.py
index 2975421e51d..8280062c3a9 100644
--- a/src/sage/groups/perm_gps/permgroup_named.py
+++ b/src/sage/groups/perm_gps/permgroup_named.py
@@ -286,6 +286,11 @@ class SymmetricGroup(PermutationGroup_symalt):
             self._gens = tuple([self.element_class(g, self, check=False)
                                 for g in gens])
 
+    def gap(self):
+        if self._libgap is None:
+            self._libgap = libgap.SymmetricGroup(self._deg)
+        return self._libgap
+
     def _gap_init_(self) -> str:
         """
         Return the string used to create this group in GAP.
@@ -768,6 +773,12 @@ class AlternatingGroup(PermutationGroup_symalt):
         """
         return "Alternating group of order %s!/2 as a permutation group" % self.degree()
 
+    def gap(self):
+        if self._libgap is None:
+            self._libgap = libgap.AlternatingGroup(self._deg)
+        return self._libgap

@mantepse mantepse requested a review from dimpase January 30, 2026 10:00
@github-actions
Copy link

github-actions bot commented Jan 30, 2026

Documentation preview for this PR (built with commit 2aad99e; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

@mantepse mantepse added the sd130 tickets of Sage Days 130 Le Teich label Jan 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dimpase dimpase Awaiting requested review from dimpase

Assignees

No one assigned

Labels

s: needs review sd130 tickets of Sage Days 130 Le Teich

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@mantepse