`sage.groups.matrix_gps`: Modularization fixes for imports #35306

mkoeppe · 2023-03-18T23:47:00Z

📚 Description

We move the ...MatrixGroup_gap classes to separate modules named ..._gap.

This is part of:

Meta-ticket: Modularize sagelib into separate distributions (pip-installable packages) sagemath-... for Sage 10.x #29705

📝 Checklist

I have made sure that the title is self-explanatory and the description concisely explains the PR.
I have linked an issue or discussion.
I have created tests covering the changes.
I have updated the documentation accordingly.

⌛ Dependencies

Depends on sage.{coding,groups}: Replace imports from sage.*.all for namespace packages #35099
Depends on Deprecate is_FiniteField etc., make sage.rings.finite_rings a namespace package #35119

…___all_for_namespace_packages' into matrix_groups_modularization

…te ..._gap module

…m .all

…nitefield' into matrix_groups_modularization

…ce of is_MatrixGroup, is_ArithmeticSubgroup

… back to _generic

codecov-commenter · 2023-03-19T07:35:10Z

Codecov Report

Patch coverage: 89.69% and project coverage change: -0.02 ⚠️

Comparison is base (c00e6c2) 88.62% compared to head (c362fb7) 88.61%.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop   #35306      +/-   ##
===========================================
- Coverage    88.62%   88.61%   -0.02%     
===========================================
  Files         2148     2155       +7     
  Lines       398855   398915      +60     
===========================================
+ Hits        353480   353484       +4     
- Misses       45375    45431      +56

Impacted Files	Coverage Δ
src/sage/groups/affine_gps/affine_group.py	`82.02% <ø> (-16.86%)`	⬇️
src/sage/groups/affine_gps/euclidean_group.py	`50.00% <ø> (-50.00%)`	⬇️
src/sage/groups/affine_gps/group_element.py	`89.38% <ø> (-5.31%)`	⬇️
src/sage/matrix/matrix_space.py	`89.07% <69.23%> (-0.55%)`	⬇️
src/sage/groups/matrix_gps/orthogonal.py	`92.85% <71.42%> (-4.62%)`	⬇️
src/sage/groups/matrix_gps/unitary.py	`82.08% <71.42%> (-9.70%)`	⬇️
src/sage/groups/matrix_gps/symplectic.py	`88.88% <77.77%> (-8.99%)`	⬇️
src/sage/groups/matrix_gps/linear.py	`84.90% <80.00%> (-6.40%)`	⬇️
src/sage/groups/matrix_gps/finitely_generated.py	`83.33% <87.50%> (-7.18%)`	⬇️
...c/sage/groups/matrix_gps/finitely_generated_gap.py	`89.28% <89.28%> (ø)`
... and 15 more

... and 28 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

…dularization

…random

SageMath version 10.0.beta7, Release Date: 2023-04-01

…ite_rings' when GF is involved, add '# optional - sage.symbolic'

…ted by mistake)

SageMath version 10.0.beta8, Release Date: 2023-04-06

kwankyu · 2023-04-14T06:20:30Z

Looks good in the first pass.

Would you put the boilerplate things like copyright notices in the new files?

…lame -M -C -w --date=format:%Y FILE.py | sort -k2'

mkoeppe · 2023-04-14T18:56:09Z

Done.

src/sage/groups/matrix_gps/matrix_group.py

src/sage/groups/matrix_gps/matrix_group_gap.py

…tors'

src/sage/groups/matrix_gps/orthogonal_gap.py

…sed on 'git blame -M -C -w --date=format:%Y FILE.py | sort -k2'

kwankyu

Thanks.

github-actions · 2023-04-17T03:41:28Z

Documentation preview for this PR is ready! 🎉
Built with commit: 1024a23

mkoeppe · 2023-04-17T06:42:25Z

Thank you!

antonio-rojas · 2023-04-27T16:32:02Z

I'm getting a test failure on Arch with system GAP 4.12.2 after this

**********************************************************************
File "/usr/lib/python3.10/site-packages/sage/groups/matrix_gps/finitely_generated_gap.py", line 124, in sage.groups.matrix_gps.finitely_generated_gap.FinitelyGeneratedMatrixGroup_gap.as_permutation_group
Failed example:
    P == Psmaller
Expected:
    False
Got:
    True
**********************************************************************

Strangely, the exact same test passed when it was in finitely_generated.py

kwankyu · 2023-04-28T05:05:13Z

To examine the failure, I tried to run tox -e docker-archlinux-latest-standard but this failed with

...
DEPRECATED: The legacy builder is deprecated and will be removed in a future release.
            BuildKit is currently disabled; enable it by removing the DOCKER_BUILDKIT=0
            environment-variable.
...
Step 16/44 : RUN sh -x -c "${BOOTSTRAP}"
 ---> Running in 3cf527175f0d
+ ./bootstrap
bootstrap:39: installing 'm4/sage_spkg_configures.m4'
/sage/build/bin/sage-bootstrap-python: error: none of python python3 python3.12 python3.11 python3.10 python3.9 python3.8 python3.7 python2.7 python3.6 python2 is a suitable Python
...

Next I tried to run ci linux workflow on my fork of sage. It also failed (actually all flavors of linux failed)

What did I wrong? I have zero experience in testing sage on linux platforms, by the way.

dimpase · 2023-04-28T11:51:30Z

It's probably a bug in tox file. Anyway, you can just run archlinux in Docker, and build Sage there the usual way.

kwankyu · 2023-04-28T12:18:03Z

Yes. This is a good opportunity for me to try sage on linux.

antonio-rojas · 2023-04-28T14:45:51Z

After some more testing, it seems to be the line

56    sage: TestSuite(G).run()

which triggers this. It is somehow interfering with the random generator status of GAP.
The test

132    sage: Psmaller.degree()                     # random

was marked as # random in c362fb7 (with no given reason), I guess the same should be done for this one.

dimpase · 2023-04-28T15:11:41Z

tagging everything #random is a bit lame. One can reset GAP's RNG before a test, like this:

gap.Reset(gap.GlobalMersenneTwister,"42")
libgap.Reset(libgap.GlobalMersenneTwister,"42")

(this is both for pexpect and libgap interfaces - probably just one of these is needed)

antonio-rojas · 2023-04-28T15:26:15Z

tagging everything #random is a bit lame. One can reset GAP's RNG before a test, like this:
gap.Reset(gap.GlobalMersenneTwister,"42")
libgap.Reset(libgap.GlobalMersenneTwister,"42")
(this is both for pexpect and libgap interfaces - probably just one of these is needed)

That's done in the Psmaller constructor already, so the difference must come from somewhere else.

antonio-rojas · 2023-04-28T15:33:50Z

Things get weirder, it depends on the order in which P and Psmaller are defined:

sage: imf = libgap.function_factory('ImfMatrixGroup')
sage: GG = imf( 12, 3 )
sage: G = MatrixGroup(GG.GeneratorsOfGroup())
sage: P = G.as_permutation_group()
sage: Psmaller = G.as_permutation_group(algorithm="smaller", seed=6)
sage: P == Psmaller
False

vs

sage: imf = libgap.function_factory('ImfMatrixGroup')
sage: GG = imf( 12, 3 )
sage: G = MatrixGroup(GG.GeneratorsOfGroup())
sage: Psmaller = G.as_permutation_group(algorithm="smaller", seed=6)
sage: P = G.as_permutation_group()
sage: P == Psmaller
True

dimpase · 2023-04-28T19:05:12Z

How about replacing the iffy test with

sage: P._libgap_().GroupHomomorphismByImages(Psmaller._libgap_()).Kernel()
Group(())

antonio-rojas · 2023-07-11T18:33:41Z

I finally figured out why this happens only when using system gap: the difference is the radiroot package, which is not installed by our gap system package. Why it has this side effect, I have no idea.

mkoeppe added 3 commits March 18, 2023 16:19

sage.groups.matrix_gps: Move ..._gap classes to separate ..._gap modules

1e9d096

Merge branch 't/34948/sage__coding_groups___replace_imports_from_sage…

3440ffe

…___all_for_namespace_packages' into matrix_groups_modularization

sage.groups.matrix_gps: Update imports of ..._gap classes

c95f132

mkoeppe added c: group theory t: refactoring labels Mar 18, 2023

mkoeppe self-assigned this Mar 18, 2023

mkoeppe changed the title ~~sage.groups.matrix_gps: Modularization fixes for imports~~ sage.groups.matrix_gps: Modularization fixes for imports Mar 18, 2023

mkoeppe added 9 commits March 18, 2023 17:25

sage.groups.matrix_gps: Move MatrixGroupElement_gap class to a separa…

d6d5747

…te ..._gap module

sage.groups.matrix_gps: Make a namespace package, replace imports fro…

b17ab88

…m .all

sage.groups.matrix_gps: Fix up imports

a074b13

src/doc/en/reference/groups/index.rst: Add the ..._gap modules

3f1d650

Merge branch 't/32664/add_sage_rings_abc_finitefield__deprecate_is_fi…

961fa72

…nitefield' into matrix_groups_modularization

sage.groups.matrix_gps: Mark doctests # optional

009fb6c

sage.groups.{affine_gps,matrix_gps}: Mark doctests # optional

7f41289

src/sage/matrix/matrix_space.py: Use try...except for import/isinstan…

12ad68a

…ce of is_MatrixGroup, is_ArithmeticSubgroup

sage.groups.matrix_gps: When ..._gap classes cannot be imported, fall…

fb55f02

… back to _generic

mkoeppe added 2 commits March 19, 2023 10:39

Merge remote-tracking branch 'upstream/develop' into matrix_groups_mo…

66f7d76

…dularization

src/doc/en/reference/groups/index.rst: Fix up

97b5480

mkoeppe requested a review from dimpase March 19, 2023 20:27

mkoeppe added the s: needs review label Mar 19, 2023

src/sage/groups/matrix_gps/finitely_generated_gap.py: Mark a doctest …

c362fb7

…random

mkoeppe requested a review from tscrim March 28, 2023 07:22

mkoeppe added 4 commits April 1, 2023 10:45

Merge tag '10.0.beta7' into matrix_groups_modularization

ce07608

SageMath version 10.0.beta7, Release Date: 2023-04-01

sage.groups.{affine_gps,matrix_gps}: Use '# optional - sage.rings.fin…

5dccde4

…ite_rings' when GF is involved, add '# optional - sage.symbolic'

src/sage/groups/matrix_gps/all__sagemath_polyhedra.py: Remove (commit…

45a332d

…ted by mistake)

Merge tag '10.0.beta8' into matrix_groups_modularization

f5a2807

SageMath version 10.0.beta8, Release Date: 2023-04-06

mkoeppe mentioned this pull request Apr 11, 2023

Adopt the “time window-based” policy for support of Python versions from NEP 29 #35403

Open

5 tasks

mkoeppe requested a review from videlec April 12, 2023 20:54

mkoeppe added 2 commits April 14, 2023 10:58

Merge branch 'develop' into matrix_groups_modularization

0592931

src/sage/groups/matrix_gps: Update/add copyright notices using 'git b…

9de361d

…lame -M -C -w --date=format:%Y FILE.py | sort -k2'

sage.groups.matrix_gps: Docstring cosmetics

bea635b

mkoeppe force-pushed the matrix_groups_modularization branch from 0aeb747 to bea635b Compare April 14, 2023 22:56

kwankyu reviewed Apr 16, 2023

View reviewed changes

src/sage/groups/matrix_gps/matrix_group.py Show resolved Hide resolved

kwankyu reviewed Apr 16, 2023

View reviewed changes

src/sage/groups/matrix_gps/matrix_group_gap.py Show resolved Hide resolved

src/sage/groups/matrix_gps/matrix_group*.py: Fix up markup of 'genera…

e15c6e8

…tors'

kwankyu reviewed Apr 17, 2023

View reviewed changes

src/sage/groups/matrix_gps/orthogonal_gap.py Show resolved Hide resolved

src/sage/groups/matrix_gps/orthogonal_gap.py: Add copyright notice ba…

1024a23

…sed on 'git blame -M -C -w --date=format:%Y FILE.py | sort -k2'

kwankyu approved these changes Apr 17, 2023

View reviewed changes

kwankyu added s: positive review and removed s: needs review labels Apr 17, 2023

vbraun merged commit 55ebb79 into sagemath:develop Apr 23, 2023
8 of 9 checks passed

mkoeppe added this to the sage-10.0 milestone Apr 23, 2023

mkoeppe removed the s: positive review label Apr 23, 2023

mkoeppe mentioned this pull request May 2, 2023

Meta-ticket: Modularize sagelib into separate distributions (pip-installable packages) sagemath-... for Sage 10.x #29705

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`sage.groups.matrix_gps`: Modularization fixes for imports #35306

`sage.groups.matrix_gps`: Modularization fixes for imports #35306

mkoeppe commented Mar 18, 2023 •

edited

codecov-commenter commented Mar 19, 2023 •

edited

kwankyu commented Apr 14, 2023

mkoeppe commented Apr 14, 2023

kwankyu left a comment

github-actions bot commented Apr 17, 2023

mkoeppe commented Apr 17, 2023

antonio-rojas commented Apr 27, 2023

kwankyu commented Apr 28, 2023 •

edited

dimpase commented Apr 28, 2023

kwankyu commented Apr 28, 2023

antonio-rojas commented Apr 28, 2023

dimpase commented Apr 28, 2023 •

edited

antonio-rojas commented Apr 28, 2023

antonio-rojas commented Apr 28, 2023

dimpase commented Apr 28, 2023

antonio-rojas commented Jul 11, 2023

sage.groups.matrix_gps: Modularization fixes for imports #35306

sage.groups.matrix_gps: Modularization fixes for imports #35306

Conversation

mkoeppe commented Mar 18, 2023 • edited

📚 Description

📝 Checklist

⌛ Dependencies

codecov-commenter commented Mar 19, 2023 • edited

Codecov Report

kwankyu commented Apr 14, 2023

mkoeppe commented Apr 14, 2023

kwankyu left a comment

Choose a reason for hiding this comment

github-actions bot commented Apr 17, 2023

mkoeppe commented Apr 17, 2023

antonio-rojas commented Apr 27, 2023

kwankyu commented Apr 28, 2023 • edited

dimpase commented Apr 28, 2023

kwankyu commented Apr 28, 2023

antonio-rojas commented Apr 28, 2023

dimpase commented Apr 28, 2023 • edited

antonio-rojas commented Apr 28, 2023

antonio-rojas commented Apr 28, 2023

dimpase commented Apr 28, 2023

antonio-rojas commented Jul 11, 2023

`sage.groups.matrix_gps`: Modularization fixes for imports #35306

`sage.groups.matrix_gps`: Modularization fixes for imports #35306

mkoeppe commented Mar 18, 2023 •

edited

codecov-commenter commented Mar 19, 2023 •

edited

kwankyu commented Apr 28, 2023 •

edited

dimpase commented Apr 28, 2023 •

edited