package modular_resolution: Split out from p_group_cohomology
Closed this issue · 180 comments
As discussed in #28711 comment:64:
We split the current p_group_cohomology spkg:
In this ticket, we create the package modular_resolution, depending on meataxe-the-C-library, that installs:
- a standalone C library
libmodres.soin$SAGE_LOCAL/lib - some executables in
$SAGE_LOCAL/bin, linked againstlibmodres - database of cohomology rings
This also fixes the build problems of p_group_cohomology.
Fixing the doctest failures of p_group_cohomology is outside of the scope of this ticket. That's #34333.
New upstream:
Depends on #34221
Depends on #34291
Upstream: Fixed upstream, in a later stable release.
CC: @simon-king-jena @dimpase @jhpalmieri @tscrim @kwankyu
Component: packages: optional
Keywords: sd111
Author: Matthias Koeppe, Dima Pasechnik, John Palmieri
Branch/Commit: c7108e9
Reviewer: John Palmieri, Matthias Koeppe, Dima Pasechnik, Travis Scrimshaw
Issue created by migration from https://trac.sagemath.org/ticket/30787
I'll start with summing up what the current p_group_cohomology spkg comprises and how the parts depend upon each other.
- the C library
libmodres, which also links againstlibmtxand thus depends on themeataxespkg, but not necessarily onsage-meataxe. - some executables linked against
libmodres - some module for Singular, that should be installed in
SAGE_LOCAL/share/singular/LIB/(that's where Singular expect modules) - some package for GAP, which by #28711 should be installed in
SAGE_LOCAL/share/gap/pkg/(that's where GAP expect packages) - some database of cohomology rings, to be installed in
SAGE_LOCAL/share/pGroupCohomology(could in principle be moved to somewhere else, but is data and thus shouldn't be in places likeSAGE_SRC) - some pip installable modules linked against
libmodresandlibmtxand also depending onsage.matrix.matrix_gfpn_densebeing built and uses some Cython headers fromsage.structure. The pip installable modules could in principle be put into the Sage src tree, analogously tosage-meataxe. 2.-5. are run time dependencies.
Obviously, 1. and 2. from the previous comment should be in the new modular_resolution spkg. But at what point should 3.-5. be installed? Currently, 5. is installed when making modular_resolution, whereas 3.-4. are installed by p_group_cohomology's spkg-install.
Would it be better to install ALL of 3.-5. by modular_resolution's spkg-install (not by the make file)? Or ALL of 3.-5. by p_group_cohomology's spkg-install? Or something else?
The python/cython modules should be left alone, do not bundle anything with it please.
Have you thought about upstreaming the Singular library?
Replying to @dimpase:
The python/cython modules should be left alone, do not bundle anything with it please.
Have you thought about upstreaming the Singular library?
No. I'm not sure if I should. It is about the computation of Dickson invariants, though. Since Singular has some capabilities of computing non-modular and modular invariant rings of finite group actions, it would somehow make sense to include it in Singular. (EDIT: I think I'll ask the Singular developers).
Concerning the analogous question on the GAP package: In this case I think it makes no sense to make it an upstream package. It calls the executables of modular_resolution, writing initial data needed for p_group_cohomology into several files.
Replying to @simon-king-jena:
Concerning the analogous question on the GAP package: In this case I think it makes no sense to make it an upstream package. It calls the executables of
modular_resolution, writing initial data needed for p_group_cohomology into several files.
An important consideration is the ability to have spkg-check - e.g. for instance you can bundle the GAP package with modular_resolution, so that you can easily write some tests (in GAP) for all of them.
Replying to @dimpase:
An important consideration is the ability to have
spkg-check- e.g. for instance you can bundle the GAP package with modular_resolution, so that you can easily write some tests (in GAP) for all of them.
Good point!
If one bundles modular_resolution together with the cohomology database (which is currently the case) and with the GAP package, one could add the following self test: Call a function from the GAP package that uses an executable from modular_resolution to create initial data for, say, the cohomology ring of the dihedral group, which is part of the database, i.e., one can verify that the newly created initial data coincides what is found in the data base.
Replying to @simon-king-jena:
Obviously, 1. and 2. from the previous comment should be in the new modular_resolution spkg. But at what point should 3.-5. be installed? Currently, 5. is installed when making modular_resolution, whereas 3.-4. are installed by p_group_cohomology's spkg-install.
Would it be better to install ALL of 3.-5. by modular_resolution's spkg-install (not by the make file)? Or ALL of 3.-5. by p_group_cohomology's spkg-install? Or something else?
parts 3.-4. are outside of the scope of this ticket.
part 5. Is it used by 1.-2.? Then it should be included here; otherwise, outside of the scope of this ticket.
Replying to @mkoeppe:
parts 3.-4. are outside of the scope of this ticket.
You argued in #28711, comment 58, that "an spkg should either be a pip-installable Python package, or it should be a non-Python package (which can install anywhere in SAGE_LOCAL)."
If I understand correctly, it means that not only the C library libmodres but also the database as well as the helpers for GAP and Singular shouldn't be installed by p_group_cohomology (whose main part IS pip-installable). So, 3.-5. should be split out as well and thus ARE in the scope of this ticket.
part 5. Is it used by 1.-2.? Then it should be included here; otherwise, outside of the scope of this ticket.
Then tell me by what source the database should be provided, if it shouldn't be p_group_cohomology.
Replying to @simon-king-jena:
Replying to @mkoeppe:
parts 3.-4. are outside of the scope of this ticket.
You argued in #28711, comment 58, that "an spkg should either be a pip-installable Python package, or it should be a non-Python package (which can install anywhere in SAGE_LOCAL)."
If I understand correctly, it means that not only the C library libmodres but also the database as well as the helpers for GAP and Singular shouldn't be installed by p_group_cohomology (whose main part IS pip-installable).
Yes, but this ticket is titled "package modular_resolution: Split out from p_group_cohomology", so it is more narrowly focused than a ticket titled "Split p_group_cohomology into its irreducible components".
Replying to @mkoeppe:
Yes, but this ticket is titled "package modular_resolution: Split out from p_group_cohomology", so it is more narrowly focused than a ticket titled "Split p_group_cohomology into its irreducible components".
I don't understand what you suggest.
Do you suggest to change the title and description of the ticket? Or do you suggest to keep the Singular and Gap helpers in p_group_cohomology and additionally to move the database from modular_cohomology (that's where the database currently is) to p_group_cohomology, even though it is basically a pip installable module? Or something else?
Replying to @simon-king-jena:
Or do you suggest to keep the Singular and Gap helpers in p_group_cohomology
Yes, that's my suggestion for this ticket - to keep the ticket simple. Follow-up tickets can take care of further restructuring.
additionally to move the database from modular_cohomology (that's where the database currently is) to p_group_cohomology
If the modular_cohomology Makefile currently installs this database, there's probably no need to change it.
If the database is part of the modular_resolution package, I would suggest to install an executable script (perhaps modres-config) that reports the install location.
Replying to @mkoeppe:
If the database is part of the
modular_resolutionpackage, I would suggest to install an executable script (perhapsmodres-config) that reports the install location.
How can this be done? I mean, I know where the database ends up. Makefile.am is
ACLOCAL_AMFLAGS = -I m4
SUBDIRS = src
dbdir = $(datadir)/pGroupCohomology
dist_db_DATA = group_cohomology_database.tar
install-data-hook:
cd $(DESTDIR)$(dbdir) && tar xf $(dist_db_DATA) && rm $(dist_db_DATA)
uninstall-hook:
rm -r $(DESTDIR)$(dbdir)
and so the database will be in $SAGE_LOCAL/share/pGroupCohomology. But I guess a script returning that path (dependent on the value of SAGE_LOCAL is not what you suggest.
Is there a standard script in the autotools ecosystem that would provide the correct value of $(dbdir) both during and after (staged) installation?
Replying to @simon-king-jena:
Replying to @mkoeppe:
If the database is part of the
modular_resolutionpackage, I would suggest to install an executable script (perhapsmodres-config) that reports the install location.How can this be done? I mean, I know where the database ends up. Makefile.am is
ACLOCAL_AMFLAGS = -I m4 SUBDIRS = src dbdir = $(datadir)/pGroupCohomology dist_db_DATA = group_cohomology_database.tar install-data-hook: cd $(DESTDIR)$(dbdir) && tar xf $(dist_db_DATA) && rm $(dist_db_DATA) uninstall-hook: rm -r $(DESTDIR)$(dbdir)and so the database will be in
$SAGE_LOCAL/share/pGroupCohomology. But I guess a script returning that path (dependent on the value ofSAGE_LOCALis not what you suggest.
Not depending on SAGE_LOCAL - the upstream package should make sense without Sage.
Rather, depending on the configured install prefix.
Like this: Create template file modres-config.in
#! /bin/sh
echo "@datadir@/pGroupCohomology"
and add to configure.ac:
AC_CONFIG_FILES([modres-config], [chmod +x modres-config])
Is there a standard script in the autotools ecosystem that would provide the correct
value of $(dbdir) both during and after (staged) installation?
No, one would not want to read from DESTDIR.
Replying to @mkoeppe:
Like this: Create template file
modres-config.in#! /bin/sh echo "@datadir@/pGroupCohomology"and add to configure.ac:
AC_CONFIG_FILES([modres-config], [chmod +x modres-config])Is there a standard script in the autotools ecosystem that would provide the correct
value of
$(dbdir)both during and after (staged) installation?No, one would not want to read from DESTDIR.
Do I understand correctly that the syntax @datadir@ in modres-config.in makes it so that the value $(datadir) is inserted during creation of the modres-config script, so that calling it later provides the final destination of the database? Well, I simply didn't know how to achieve this and is what I meant by a "standard script in the autotools ecosystem".
To be clear: Do you suggest that this script will be called by the Python/Cython modules to find out the location of the database?
Replying to @simon-king-jena:
Do I understand correctly that the syntax
@datadir@inmodres-config.inmakes it so that the value$(datadir)is inserted during creation of themodres-configscript, so that calling it later provides the final destination of the database?
That's right. ./configure will create the file modres-config. There are, of course, many other ways to do it, like generating this simple script in an install-hook.
Well, I simply didn't know how to achieve this and is what I meant by a "standard script in the autotools ecosystem".
Ok, great; but note that it does not do anything about DESTDIR (and should not).
To be clear: Do you suggest that this script will be called by the Python/Cython modules to find out the location of the database?
Yes.
Replying to @mkoeppe:
To be clear: Do you suggest that this script will be called by the Python/Cython modules to find out the location of the database?
Yes.
Is this ticket only about creating a new modular_resolution spkg, or also to modify p_group_cohomology to use the new split-off spkg? Or shall the latter be done on a new ticket?
While we are at it: Is there a recommended way (at least recommended in Sage) to read from the output of system calls in Python? os.popen, or subprocess.Popen or something else?
Replying to @simon-king-jena:
Is this ticket only about creating a new
modular_resolutionspkg, or also to modifyp_group_cohomologyto use the new split-off spkg? Or shall the latter be done on a new ticket?
Both
While we are at it: Is there a recommended way (at least recommended in Sage) to read from the output of system calls in Python?
os.popen, orsubprocess.Popenor something else?
I just learned the hard way how to write this portably for Python >= 3.6 (all our currently supported Python versions): Take a look at the most recent version (after #30758) of src/sage/features/__init.py__ (package_systems).
from subprocess import run, CalledProcessError, PIPE
try:
proc = run('sage-guess-package-system', shell=True, stdout=PIPE, stderr=PIPE, universal_newlines=True, check=True)
_cache_package_systems = [PackageSystem(proc.stdout.strip())]
except CalledProcessError:
pass
Why would one want to create a custom script modres-config instead of hooking this up on pkg-config ?
(I can show you the very easy autoconf/automake magic for this, I've done this for many projects used by Sage)
Then the Python interface is already there (with Python module pkgconfig - see e.g. src/sage/env.py)
Replying to @dimpase:
Why would one want to create a custom script
modres-configinstead of hooking this up onpkg-config?
I have no preference.
(I can show you the very easy autoconf/automake magic for this, I've done this for many projects used by Sage)
Please do!
We were discussing how to advertise the install location of the data files. Pkgconfig for the library would also be nice of course
Replying to @simon-king-jena:
(I can show you the very easy autoconf/automake magic for this, I've done this for many projects used by Sage)
Please do!
In particular, I have no idea how to make pkgconfig aware of an autotoolized library: pkgconfig.list_all() contains neither libmtx nor mtx nor meataxe nor modres nor libmodres nor modular_resolution, although meataxe and modular_resolution are installed.
Replying to @simon-king-jena:
Replying to @simon-king-jena:
(I can show you the very easy autoconf/automake magic for this, I've done this for many projects used by Sage)
Please do!
In particular, I have no idea how to make pkgconfig aware of an autotoolized library:
pkgconfig.list_all()contains neitherlibmtxnormtxnormeataxenormodresnorlibmodresnormodular_resolution, although meataxe and modular_resolution are installed.
As an example, have a look at e.g. cddlib/cddlib#40
(It changes a bit more than just adds cddlib.pc (adds some GMP-related stuff), so skip these parts)
Namely,
Makefile.amneeded
pkgconfigdir = $(libdir)/pkgconfig
pkgconfig_DATA = cddlib.pc
-
cddlib.pc.in(a template) needed to be created -
cddlib.pchad to be added to the list inAC_CONFIG_FILES
After all this, ./configure will fill in cddlib.pc.in and create cddlib.pc, which is then installed by make install.
Then Python pkgconfig module will be aware of that .pc file, no problem.
By the way, you can have a custom variable in .pc file - in your case, say, the path to the database - and request it using
pkg-config --variable=... ... calls (and from Python pkgconfig in a similar way)
Thank you, Dima!
I wonder: Should I add pkg-config to SharedMeatAxe as well? After all, meataxe uses arithmetic tables that are of course stored in some default folder, and currently its location is presented by an environment variable. This variable is needed by the meataxe executables (that are also part of the package), whereas libmtx uses a C variable for the same purpose.
I took this design from the original MeatAxe from which I forked off SharedMeatAxe, but I've never been happy with it.
On the other hand, I'm not sure if pkg-config would be the right tool in this case: The location of the tables is in fact not determined by the autotoolised build system, but it is determined at run time -- and moreover the arithmetic tables are first created by spkg-postinst, not by the makefile of meataxe.
Replying to @simon-king-jena:
Thank you, Dima!
I wonder: Should I add pkg-config to SharedMeatAxe as well? After all, meataxe uses arithmetic tables that are of course stored in some default folder, and currently its location is presented by an environment variable. This variable is needed by the meataxe executables (that are also part of the package), whereas libmtx uses a C variable for the same purpose.
It certainly simplifies things, to be able to use pkg-config to check the library. E.g. imagine meataxe library available as a Debian package, and then naturally one would want to use it rather than to build Sage's one...
As to location of the tables/databases, naturally, there is no standard. Although I imagine there are defaults set in libmtx and in meataxe executables.
I took this design from the original
MeatAxefrom which I forked offSharedMeatAxe, but I've never been happy with it.On the other hand, I'm not sure if pkg-config would be the right tool in this case: The location of the tables is in fact not determined by the autotoolised build system, but it is determined at run time -- and moreover the arithmetic tables are first created by spkg-postinst, not by the makefile of meataxe.
How is this determination happening at runtime? Checking an environment variable? Or there is a default value?
Replying to @dimpase:
It certainly simplifies things, to be able to use pkg-config to check the library. E.g. imagine meataxe library available as a Debian package, and then naturally one would want to use it rather than to build Sage's one...
However, the location of the arithmetic tables does not depend on who built it. If the tables can not be found in the location specified by the environment variable MTXLIB resp. the C variable MtxLibDir, then it is attempted to read from pwd. And if this fails, it is attempted create new tables.
As to location of the tables/databases, naturally, there is no standard. Although I imagine there are defaults set in libmtx and in meataxe executables.
The default is the current directory (so, better not use the default!).
How is this determination happening at runtime? Checking an environment variable? Or there is a default value?
Actually I don't recall if there is a default value of the variable. You as a user of meataxe executables on the command line are supposed to assign a value to the environment variable, and you as a programmer of code linked against libmtx are supposed to assign a value to the C variable. Sage does both automatically, if you import from sage.libs.meataxe.
So, the user (Sage, in this case) has full control. If one fails to assign a value, then the current directory is used, or an error results (I don't recall).
Replying to @dimpase:
By the way, you can have a custom variable in .pc file - in your case, say, the path to the database - and request it using
pkg-config --variable=... ...calls (and from Pythonpkgconfigin a similar way)
This is a great point that I had forgotten about -- so pkg-config is a great solution for advertising the location of the data files too.
I would suggest deferring the discussion about meataxe to a separate ticket
Replying to @mkoeppe:
I would suggest deferring the discussion about meataxe to a separate ticket
I wanted to know whether a new ticket makes sense at all, before opening it.
Changed keywords from none to sd111
Hoping we can make progress on this ticket this week - https://wiki.sagemath.org/days111
Setting new milestone based on a cursory review of ticket status, priority, and last modification date.
while working on #31404, and trying p_group_cohomology in the new setup, I saw that Singular has renamed poly.lib to polylib.lib.
This causes errors in the doctests, with Singular 4.2. Maybe a special ticket for such things? Also
the format of dickson.lib is old:
> LIB "dickson.lib";
// ** loaded /mnt/opt/Sage/sage-dev/local/share/singular/LIB/dickson.lib
// ** library dickson.lib has old format. This format is still accepted,
// ** but for functionality you may wish to change to the new
// ** format. Please refer to the manual for further information.
// ** loaded /usr/bin/../share/singular/LIB/polylib.lib (4.2.0.0,Dec_2020)
// ** redefining ringweights (LIB "dickson.lib";)
// ** redefining ringweights (LIB "dickson.lib";)
// ** loaded /usr/bin/../share/singular/LIB/ring.lib (4.1.2.0,Feb_2019)
// ** loaded /usr/bin/../share/singular/LIB/primdec.lib (4.2.0.0,Mar_2021)
// ** loaded /usr/bin/../share/singular/LIB/absfact.lib (4.1.2.0,Feb_2019)
// ** loaded /usr/bin/../share/singular/LIB/triang.lib (4.1.2.0,Feb_2019)
// ** loaded /usr/bin/../share/singular/LIB/matrix.lib (4.1.2.0,Feb_2019)
// ** loaded /usr/bin/../share/singular/LIB/nctools.lib (4.1.2.0,Feb_2019)
// ** loaded /usr/bin/../share/singular/LIB/random.lib (4.1.2.0,Feb_2019)
// ** loaded /usr/bin/../share/singular/LIB/elim.lib (4.1.2.0,Feb_2019)
// ** loaded /usr/bin/../share/singular/LIB/inout.lib (4.1.2.0,Feb_2019)
// ** loaded /usr/bin/../share/singular/LIB/general.lib (4.1.2.0,Feb_2019)
>
For what it is worth, I am attaching a patch for p_group_cohomology/spkg-install.in
and a patch (include_dir.patch) to be installed in p_group_cohomology/patches/.
I was able to build the spkg with these changes.
patch for p_group_cohomology/spkg-install.in
Attachment: p_group_cohomology.patch.gz
Attachment: include_dir.patch.gz
patch to install in p_group_cohomology/patches
Here's a beginning
New commits:
f941152 | build/pkgs/modular_resolution: New, split from p_group_cohomology |
Author: Matthias Koeppe, ...
Branch pushed to git repo; I updated commit sha1. New commits:
364f839 | build/pkgs/{p_group_cohomology,modular_resolution}: First attempt at dependencies |
Branch pushed to git repo; I updated commit sha1. New commits:
fce05ca | build/pkgs/p_group_cohomology/spkg-install.in: Remove installation of modular_resolution |
untested, feel free to continue. getting late here.
OK, this builds - but the GAP helper files aren't ending in the correct place. Thus this fix.
Still need to sort of dickson.lib, as the system-wide Singular doesn't find it.
New commits:
0b92e84 | move installing GAP and Singular files to spkg-postinstall |
Changed author from Matthias Koeppe, ... to Matthias Koeppe, Dima Pasechnik
OK, the following tells Singular where to look for:
dima@hilbert /mnt/opt/Sage/sage-dev $ export SINGULARPATH=`pwd`/local/share/singular/LIB
dima@hilbert /mnt/opt/Sage/sage-dev $ ./sage --singular
SINGULAR / Development
A Computer Algebra System for Polynomial Computations / version 4.2.1
0<
by: W. Decker, G.-M. Greuel, G. Pfister, H. Schoenemann \ May 2021
FB Mathematik der Universitaet, D-67653 Kaiserslautern \
> LIB "dickson.lib";
// ** loaded /mnt/opt/Sage/sage-dev/local/share/singular/LIB/dickson.lib
// ** library dickson.lib has old format. This format is still accepted,
// ** but for functionality you may wish to change to the new
// ** format. Please refer to the manual for further information.
? cannot open `poly.lib`
// ** loaded /usr/bin/../share/singular/LIB/general.lib (4.1.2.0,Feb_2019)
...
And we're running into an old name for polylib.lib.
Branch pushed to git repo; I updated commit sha1. New commits:
45e6779 | patch to change poly.lib -> polylib.lib |
OK, so now we are in a reasonable shape; one doctest fails due to deprecation of set_default_prec(), used in upstream/p_group_cohomology-3.3.2/pGroupCohomology-3.3.2/pGroupCohomology/barcode.py,
the rest work.
And we need to sort out how to deal with SINGULARPATH - cf. comment:48.
Replying to @dimpase:
And we need to sort out how to deal with
SINGULARPATH- cf. comment:48.
Don't try to adjust SINGULARPATH, just load the library from the absolute pathname
Likewise for the gap helper, don't try to install it in the GAP library, just load it from the absolute pathname
I installed this on OS X, although the first time failed in building modular_resolution:
./modular_resolution.h:29:10: fatal error: 'meataxe.h' file not found
#include "meataxe.h"
^~~~~~~~~~~
1 error generated.
Maybe add meataxe to the dependencies file for modular_resolution?
After building everything, the test suite failed almost immediately with
Running the test suite for p_group_cohomology-3.3.2.p0...
make[3]: *** No rule to make target `check'. Stop.
********************************************************************************
Error testing modular_resolution-1.1
********************************************************************************
real 0m0.134s
user 0m0.016s
sys 0m0.025s
************************************************************************
Error testing package p_group_cohomology-3.3.2.p0
************************************************************************
The package has an extensive test suite, so if we can get it to pass tests, that should be a strong indication that everything is working.
Replying to @jhpalmieri:
Maybe add
meataxeto the dependencies file formodular_resolution?
Here it is. I had forgotten to add the file
The documentation doesn't build:
Running Sphinx v4.4.0
Extension error:
Could not import extension inventory_builder (exception: No module named 'inventory_builder')
make[3]: *** [html] Error 2
Patching conf.py fixed it:
diff --git a/pGroupCohomology-3.3.2/doc/source/conf.py b/pGroupCohomology-3.3.2/doc/source/conf.py
index e24ceb6..ee16257 100644
--- a/pGroupCohomology-3.3.2/doc/source/conf.py
+++ b/pGroupCohomology-3.3.2/doc/source/conf.py
@@ -34,8 +34,8 @@ sys.path.append(os.path.join(SAGE_SRC, "sage_setup", "docbuild", "ext"))
# Add any Sphinx extension module names here, as strings. They can be
# extensions coming with Sphinx (named 'sphinx.ext.*') or your custom
# ones.
-extensions = ['inventory_builder',
- 'sage_autodoc',
+extensions = ['sage_docbuild.ext.inventory_builder',
+ 'sage_docbuild.ext.sage_autodoc',
'sphinx.ext.intersphinx',
'sphinx.ext.todo']but I wonder if the p_group_cohomology conf.py should just point to Sage's conf.py and use that.
testsuite for modular_resolution (created in the latest commit) passes.
With p_group_cohomology, somehow I can't uninstall it, for some reason.
New commits:
981836e | split spkg-check between corresponding packages |
Doing
export SINGULARPATH=`pwd`/local/share/singular/LIB
and after some fighting ./configure and using ./sage -f instead of make, I was able to rebuild p_group_cohomology and ran its testsuite. 2 failures - one due to deprecated set_default_prec(), clear how to fix, and another
[p_group_cohomology-3.3.2.p0] File "pGroupCohomology-3.3.2/pGroupCohomology/cohomology.pyx", line 993, in pGroupCohomology.cohomology.is_filter_regular
[p_group_cohomology-3.3.2.p0] Failed example:
[p_group_cohomology-3.3.2.p0] is_filter_regular(H.relation_ideal(), F[0])
[p_group_cohomology-3.3.2.p0] Expected:
[p_group_cohomology-3.3.2.p0] False
[p_group_cohomology-3.3.2.p0] Got:
[p_group_cohomology-3.3.2.p0] [0]
I am not sure about. Looks like a genuine change.
Branch pushed to git repo; I updated commit sha1. New commits:
2b0096c | doctest patches |
more doctest etc patches for the package. This reduces the noise, but there are still quite a lot of discrepancies, some of which look serious.
Also,
sage -t --warn-long 62.5 --random-seed=230138939486715312795148381621394057005 src/sage/tests/modular_group_cohomology.py
**********************************************************************
File "src/sage/tests/modular_group_cohomology.py", line 55, in sage.tests.modular_group_cohomology
Failed example:
ascii_art(H.bar_code('LowerCentralSeries')[2])# optional - p_group_cohomology
Expected:
*
*-*
*-*
*
Got:
*-*
*-*
*-*
*-*
*
**********************************************************************
1 item had failures:
1 of 18 in sage.tests.modular_group_cohomology
[17 tests, 1 failure, 3.71 s]
John, do you have an idea about test failures?
First of all: Thank you very much for working on it! I am currently buried in exam grading and unable to do more interesting stuff.
Both the failure at is_filter_regular(H.relation_ideal(), F[0]) and H.bar_code('LowerCentralSeries') look very serious to me. So, I should try and build stuff, and try to analyse the failures.
My question: It looks to me as if the branch introduces patches -- I guess these should eventually be included in the next version of the p_group_cohomology package, i.e., should be pushed to the github repository of the package. But how? Could you tell me a workflow?
Best regards,
Simon
Hi Simon, patches are a bit messy, as they are against the package, which has a different directory structure. So a straight git apply <patch-file> doesn't work in the upstream directory, one needs to use patch -pX with different values of X for different files.
Don't worry about this, I'll push a branch upstream with these changes applied.
After that, for the new release we'd also need to sort out packaging of GAP and Singular files, as Matthias
says. But it's certainly most important to fix the maths issues!
It's already possible to build and test this branch, with workaround in comment:62
I've collected the patches for upstream in https://github.com/sagemath/p_group_cohomology/tree/wip_trac_30787
see also
sagemath/p_group_cohomology#6
Upstream: Fixed upstream, in a later stable release.
Do I understand correctly that the only place (other than install scripts) that knows about the installation location of the database is set_local_sources (
https://github.com/sagemath/p_group_cohomology/blob/master/src/pGroupCohomology/factory.py#L617)?
New commits:
bd14496 | build/pkgs/modular_resolution/spkg-install.in: Use sdh_make_install, remove redundant error handling |
Branch pushed to git repo; I updated commit sha1. New commits:
53198c8 | build/pkgs/modular_resolution/SPKG.rst: Update for separate package |
This should fix docbuilding.
New commits:
7b994c7 | trac 30787: fix docbuilding. Mainly use Sage's conf.py. |
Changed author from Matthias Koeppe, Dima Pasechnik to Matthias Koeppe, Dima Pasechnik, John Palmieri
Branch pushed to git repo; I updated commit sha1. New commits:
8768aa4 | upstream patches, new version |
OK, refactoring done, what remains are maths errors.
Replying to @dimpase:
OK, refactoring done, what remains are maths errors.
Thank you! So, what's the plan now? I suppose I have to pull from the upstream repository to which you posted the new version, and then try to install it on my laptop, so that I can see what is happening on the mathematical side.
Question: Has there been a recent Singular upgrade in Sage? If so: What version of Sage?
I just tried to pull, but I got permission denied. It could be that I am on a new laptop now. So, how can I convince git that I have permission to pull and push? Both for the upstream repository and for the sage trac please.
you probably don't need the upstream repo now. (for github (and for git trac) you might need to create a new ssh keypair, some old keys are now deemed too insecure)
Just build the branch of this ticket.
Singular now is 4.2.0 or 1, don't recall. (away from kbd now)
You can also run this branch in GitPod (the rightmost button in Status Badges above)
You'll get a VM with this branch pulled (but without p_group_cohomology built, so you'd need to do
make p_group_cohomology followed up by make build in the terminal window there). Takes 5-10 minutes to start.
You'd need to be authenticated to GitHub, I guess, to use it.
Oops, it seems that my latest change to the package created a circular import (only detected from a fresh build it seems)
See sagemath/p_group_cohomology@3a7a043
One cannot import a package inside itself, I gather, but without this I can't
call os.path.dirname(pGroupCohomology.__file__). Any way out here?
I guess, just use __file__ in these calls, and remove import statements.
yes