Downgrade broken optional packages to experimental for Sage 9.2

[mkoeppe]

see:

• Meta-ticket: Fix optional and experimental packages #29900 Meta-ticket: Fix optional and experimental packages for Sage 9.2

Downgrading two optional packages that fail to build on many platforms, as can be seen in https://github.com/mkoeppe/sage/actions/runs/296478189

Depends on #28711

CC: @jhpalmieri @dimpase @slel @simon-king-jena

Component: packages: optional

Author: Matthias Koeppe

Branch/Commit: 674523c

Reviewer: Dima Pasechnik

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

[vbraun]

comment:2

I'd say a broken optional package is by definition not a blocker...

[mkoeppe]

Branch: u/mkoeppe/downgrade_broken_optional_packages_to_experimental_for_sage_9_2

[mkoeppe]

New commits:

402d45f

build/pkgs/{deformation,p_group_cohomology}/type: Set to experimental

[mkoeppe]

Commit: 402d45f

[mkoeppe]

Author: Matthias Koeppe

[dimpase]

comment:7

Simon, what is the current status of your package?

[simon-king-jena]

comment:8

Replying to @dimpase:

Simon, what is the current status of your package?

I thought that it works. As far as I recall, when I last checked, it did.

Anyway, I plan to give a talk on the package during the next SageDays, so, I am motivated to fix it. Can you tell me what problems arose?

[simon-king-jena]

comment:9

Replying to @simon-king-jena:

Replying to @dimpase:

Simon, what is the current status of your package?

I thought that it works. As far as I recall, when I last checked, it did.

Anyway, I plan to give a talk on the package during the next SageDays, so, I am motivated to fix it. Can you tell me what problems arose?

PS: By "my package", I refer to p_group_cohomology. There is "my other package", namely SharedMeatAxe, which is supposed to work and has recently also been fixed on cygwin (see #29152

[dimpase]

comment:10

Here are results of a local run, with 9.2.beta14 - that's newer than this ticket, but a bit older than the current rc - they match errors seen in the logs in the ticket description.
They appear to be a result of GAP files installed in a weird place, namely:

$SAGE_LOCAL/lib/python3.7/site-packages/$SAGE_LOCAL/share/sage/ext/gap/modular_cohomology/

rather than $SAGE_LOCAL/share/sage/ext/gap/modular_cohomology/. So I see

sage -t --random-seed=0 src/sage/tests/modular_group_cohomology.py
**********************************************************************
File "src/sage/tests/modular_group_cohomology.py", line 10, in sage.tests.modular_group_cohomology
Failed example:
    from pGroupCohomology import CohomologyRing   # optional - p_group_cohomology
Exception raised:
    Traceback (most recent call last):
      File "/home/scratch2/dimpase/sage/sage/local/lib64/python3.7/site-packages/sage/doctest/forker.py", line 720, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/home/scratch2/dimpase/sage/sage/local/lib64/python3.7/site-packages/sage/doctest/forker.py", line 1145, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.tests.modular_group_cohomology[0]>", line 1, in <module>
        from pGroupCohomology import CohomologyRing   # optional - p_group_cohomology
      File "/home/scratch2/dimpase/sage/sage/local/lib64/python3.7/site-packages/pGroupCohomology/__init__.py", line 2466, in <module>
        from pGroupCohomology.factory import CohomologyRing
      File "/home/scratch2/dimpase/sage/sage/local/lib64/python3.7/site-packages/pGroupCohomology/factory.py", line 43, in <module>
        from pGroupCohomology.auxiliaries import coho_options, coho_logger, safe_save, _gap_reset_random_seed, gap, singular, Failure
      File "/home/scratch2/dimpase/sage/sage/local/lib64/python3.7/site-packages/pGroupCohomology/auxiliaries.py", line 411, in <module>
        gap.Read(os.path.join(SAGE_SHARE,'sage','ext','gap','modular_cohomology','GapMaxels.g'))
      File "sage/libs/gap/element.pyx", line 2525, in sage.libs.gap.element.GapElement_Function.__call__ (build/cythonized/sage/libs/gap/element.c:19780)
        sig_on()
    sage.libs.gap.util.GAPError: Error, file "/home/scratch2/dimpase/sage/sage/local/share/sage/ext/gap/modular_cohomology/GapMaxels.g" must exist and be readable

(and more failing tests due to this, as you imagine)
So the package looks for its GAP files in $SAGE_LOCAL/share/sage/ext/gap/modular_cohomology/

Perhaps Matthias knows the reason for this.

[dimpase]

comment:11

It might be due to the packages now being built using Python wheels, and everything in the package goes to the site-packages/ of the Python, rather than to the local GAP.

[simon-king-jena]

comment:12

Replying to @dimpase:

It might be due to the packages now being built using Python wheels, and everything in the package goes to the site-packages/ of the Python, rather than to the local GAP.

Probably.

So, tell me: is there an environment variable or a Sage variable that provides the location of GAP's package data?

Since this is an actual bug: Should it be fixed here (although this ticket is about downgrading several packages rather then fixing them) or on a new ticket?

[simon-king-jena]

comment:13

Really unfortunate that the Cython code of p_group_cohomology has not been made optional Cython files in the Sage library, because then it would be easy to fix without upgrading the upstream source ball.

[simon-king-jena]

comment:14

PS:

Replying to @dimpase:

They appear to be a result of GAP files installed in a weird place, namely:

$SAGE_LOCAL/lib/python3.7/site-packages/$SAGE_LOCAL/share/sage/ext/gap/modular_cohomology/

It is indeed weird that $SAGE_LOCAL appears twice in this path. In the sources, if I see that correctly, this path appears only once, namely setup.py (data_files=[(os.path.join(SAGE_SHARE,"sage","ext","gap","modular_cohomology"))

So, if I understand correctly what the error says, GAP does indeed try to find the GAP code in that folder, and the real question is why the code hasn't been installed there during package installation.

[simon-king-jena]

comment:15

Replying to @simon-king-jena:

Replying to @dimpase:

They appear to be a result of GAP files installed in a weird place, namely:

$SAGE_LOCAL/lib/python3.7/site-packages/$SAGE_LOCAL/share/sage/ext/gap/modular_cohomology/

It is indeed weird that $SAGE_LOCAL appears twice in this path. In the sources, if I see that correctly, this path appears only once, namely setup.py (data_files=[(os.path.join(SAGE_SHARE,"sage","ext","gap","modular_cohomology"))

I stand corrected: There are some places in the file auxiliaries.py, something like gap.Read(os.path.join(SAGE_SHARE,'sage','ext','gap','modular_cohomology','GapMaxels.g')). However, $SAGE_SHARE used only once in the path.

A clean solution would be to have a Sage or an environment variable giving the location of code of Gap extensions.

[mkoeppe]

comment:16

For p_group_cohomology, see ticket #28711

[simon-king-jena]

comment:17

Replying to @mkoeppe:

For p_group_cohomology, see ticket #28711

OK, I'll post fixes there.

[dimpase]

comment:18

Replying to @simon-king-jena:

Replying to @simon-king-jena:

Replying to @dimpase:

They appear to be a result of GAP files installed in a weird place, namely:

$SAGE_LOCAL/lib/python3.7/site-packages/$SAGE_LOCAL/share/sage/ext/gap/modular_cohomology/

It is indeed weird that $SAGE_LOCAL appears twice in this path. In the sources, if I see that correctly, this path appears only once, namely setup.py (data_files=[(os.path.join(SAGE_SHARE,"sage","ext","gap","modular_cohomology"))

I stand corrected: There are some places in the file auxiliaries.py, something like gap.Read(os.path.join(SAGE_SHARE,'sage','ext','gap','modular_cohomology','GapMaxels.g')). However, $SAGE_SHARE used only once in the path.

A clean solution would be to have a Sage or an environment variable giving the location of code of Gap extensions.

You can call GAP and examine GAPInfo.RootPaths. When looking for a package, GAP appends pkg/ to each directory in this list, cf. 9.2 GAP Root Directories in its manual.

IMHO, Python wheels are build to be relocateable, so it's futile to try to install GAP
data via setup.py. I'd say, install a GAP package separately, not from setup.py from spkg-install. By the way, GAP now has its own package manager. Maybe you can use it (https://github.com/gap-packages/PackageManager) - it's in gap_packages, I think.

[mkoeppe]

comment:19

Replying to @simon-king-jena:

Really unfortunate that the Cython code of p_group_cohomology has not been made optional Cython files in the Sage library, because then it would be easy to fix without upgrading the upstream source ball.

Yes, I would hope that we can make this improvement to the source structure in Sage 9.3. Similar to what has been done in the 9.2 cycle with giacpy_sage and sage_brial.

[simon-king-jena]

comment:20

Replying to @dimpase:

Here are results of a local run, with 9.2.beta14 - that's newer than this ticket, but a bit older than the current rc - they match errors seen in the logs in the ticket description.
They appear to be a result of GAP files installed in a weird place, namely:

$SAGE_LOCAL/lib/python3.7/site-packages/$SAGE_LOCAL/share/sage/ext/gap/modular_cohomology/

rather than $SAGE_LOCAL/share/sage/ext/gap/modular_cohomology/. So I see

Note that apparently $SAGE_LOCAL/share/sage is no more, it ceased to exist, it is an ex-folder.

[simon-king-jena]

comment:21

Replying to @mkoeppe:

Replying to @simon-king-jena:

Really unfortunate that the Cython code of p_group_cohomology has not been made optional Cython files in the Sage library, because then it would be easy to fix without upgrading the upstream source ball.

Yes, I would hope that we can make this improvement to the source structure in Sage 9.3. Similar to what has been done in the 9.2 cycle with giacpy_sage and sage_brial.

Can you elaborate? At some point in the past, I suggested to change the p_group_cohomology spkg as follows:

• Its Python and Cython code should go into the Sage library, the Cython files being optional extension modules.
• Its .c files should remain an spkg, that also is the dependency of the optional extension modules.
• I now suggest, as SAGE_LOCAL/share/sage has gone, to keep the .g files in the package, but install them into SAGE_LOCAL/share/p_group_cohomology

Do you suggest this, or something else?

It used to be a problem that the documentation of the optional Cython modules would not be built. Is there a way around?

EDIT: Is it possible to advise Sage's test suite to only run the tests of a particular file if a certain optional package is installed? Or is it still only possible for individual tests? I'd find it extremely awkward to mark each individual doctest as # optional: p_group_cohomology.

[mkoeppe]

comment:22

Replying to @simon-king-jena:

Replying to @mkoeppe:

Replying to @simon-king-jena:

Really unfortunate that the Cython code of p_group_cohomology has not been made optional Cython files in the Sage library, because then it would be easy to fix without upgrading the upstream source ball.

Yes, I would hope that we can make this improvement to the source structure in Sage 9.3. Similar to what has been done in the 9.2 cycle with giacpy_sage and sage_brial.

Can you elaborate? At some point in the past, I suggested to change the p_group_cohomology spkg as follows:

• Its Python and Cython code should go into the Sage library, the Cython files being optional extension modules.

Yes

• Its .c files should remain an spkg, that also is the dependency of the optional extension modules.

Yes, in particular if that C library can also be used outside of Sage...

• I now suggest, as SAGE_LOCAL/share/sage has gone, to keep the .g files in the package, but install them into SAGE_LOCAL/share/p_group_cohomology

You could either ship them with that spkg that provides the C library and then install it in SAGE_LOCAL/share/p_group_cohomology (or, more precisely, in the configured install prefix of your C library.

Or, if you want to ship it with sagelib, then (as I commented in #28711) these files should be put alongside the Python modules. sagelib and other Python packages have no business writing into $SAGE_LOCAL.

[simon-king-jena]

comment:23

Replying to @mkoeppe:

• Its Python and Cython code should go into the Sage library, the Cython files being optional extension modules.

Yes

OK, that was what I have suggested in the past, but what was refused by other developers.

Crucial point, from my point of view: Documentation and doc tests, as explained in my other posts. Are these problems of optional extension modules solved?

• Its .c files should remain an spkg, that also is the dependency of the optional extension modules.

Yes, in particular if that C library can also be used outside of Sage...

How do you define "can"? David Green, the original author of the c-code (that originally hasn't even been a library but only a couple of executables) has used them via command line, but I think he is the only one who ever did.

I'd say: I do not intend to ever publish the c-code outside of the Sage ecosystem.

• I now suggest, as SAGE_LOCAL/share/sage has gone, to keep the .g files in the package, but install them into SAGE_LOCAL/share/p_group_cohomology

You could either ship them with that spkg that provides the C library and then install it in SAGE_LOCAL/share/p_group_cohomology (or, more precisely, in the configured install prefix of your C library.

That's what is currently happening.

Or, if you want to ship it with sagelib,

I don't.

then (as I commented in #28711) these files should be put alongside the Python modules. sagelib and other Python packages have no business writing into $SAGE_LOCAL.

[mkoeppe]

comment:24

Replying to @simon-king-jena:

Replying to @mkoeppe:

• Its .c files should remain an spkg, that also is the dependency of the optional extension modules.

Yes, in particular if that C library can also be used outside of Sage...

How do you define "can"? David Green, the original author of the c-code (that originally hasn't even been a library but only a couple of executables) has used them via command line, but I think he is the only one who ever did.

This version of "can" is good enough.

I'd say: I do not intend to ever publish the c-code outside of the Sage ecosystem.

• I now suggest, as SAGE_LOCAL/share/sage has gone, to keep the .g files in the package, but install them into SAGE_LOCAL/share/p_group_cohomology

You could either ship them with that spkg that provides the C library and then install it in SAGE_LOCAL/share/p_group_cohomology (or, more precisely, in the configured install prefix of your C library.

That's what is currently happening.

As long as you build this library using standard tools (preferably autotools) instead of Sage-specific scripts, and the C library does not depend on the Sage library, this is all fine.

[mkoeppe]

comment:25

In any case, I don't think it's realistic to fix these issues in Sage 9.2.
So let's use the present ticket to update the type of these two packages.

[dimpase]

comment:26

I think a hotfix for p_group_cohomology, to install GAP files into the right place, using SAGE_LOCAL, despite being blasphemy in the Temple of Python, can be done very quickly.

Just add sdh_install call to spkg-install, which will put them where the package expects.

[dimpase]

comment:27

after doing ln -sf $SAGE_LOCAL/lib/python3.8/site-packages/$SAGE_LOCAL/share/sage/ext/gap/modular_cohomology $SAGE_LOCAL/share/sage/ext/gap/modular_cohomology
(if it's python3.X, there should be python3.X there)

GAP files load, but Singular do not load

    sage.interfaces.singular.SingularError: Singular error:
       ? cannot open dickson.lib
       ? error occurred in or before STDIN line 15: `LIB "dickson.lib";`

So this is a similar to GAP files problem, only with Singular...

[simon-king-jena]

comment:28

Replying to @dimpase:

I think a hotfix for p_group_cohomology, to install GAP files into the right place, using SAGE_LOCAL, despite being blasphemy in the Temple of Python, can be done very quickly.

Just add sdh_install call to spkg-install, which will put them where the package expects.

See my comment 39 at #28711: Apparently Sage prepends the absolute path os.path.join(SAGE_SHARE,"sage","ext","gap","modular_cohomology") with $SAGE_DESTDIR. Same problem with the Singular LIB.

So, I guess the real hotfix is to tell me: How should I provide a RELATIVE path in which to install the GAP respectively Singular code? Would it just work to write

  data_files=[ ( os.path.join( "share", "gap", "pkg", "modular_cohomology" ),
                 [ os.path.join( "pGroupCohomology", "GapMaxels.g" ),
                   os.path.join( "pGroupCohomology", "GapMB.g" ),
                   os.path.join( "pGroupCohomology", "GapSgs.g" ) ] ),
               ( os.path.join( "share", "singular", "LIB" ),
                 [ os.path.join( "pGroupCohomology","dickson.lib" ) ] ) ]

?

EDIT: I'd also need to change the places in auxiliaries.py, because currently the install path for the GAP helpers is SAGE_SHARE/sage/ext/gap/modular_cohomology, not SAGE_SHARE/gap/pkg/modular_cohomology. But SAGE_SHARE/sage has gone.

[dimpase]

comment:29

can we get p_group_cohomology off the branch here?

[mkoeppe]

Dependencies: #28711

[sagetrac-git]

Changed commit from 402d45f to 674523c

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

4f5a810	Upgrade p_group_cohomology to version 3.3.2
de8adfa	Revert a change to "dependencies" of p_group_cohomology
cf60884	Add upstream_url to p_group_cohomology
ae50157	p_group_cohomology's upstream changed by some new commits
bd34c47	build/pkgs/p_group_cohomology/checksums.ini: Fix upstream_url
245560d	Fix docbuild of p_group_cohomology
f4e5f6a	Fix a doctest in sage/tests/modular_cohomology.py
674523c	build/pkgs/deformation/type: Downgrade to experimental

[dimpase]

comment:33

ok

[dimpase]

Reviewer: Dima Pasechnik

[vbraun]

Changed branch from u/mkoeppe/downgrade_broken_optional_packages_to_experimental_for_sage_9_2 to 674523c