sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.43k stars 479 forks source link

Cubic Hecke Algebras #29717

Closed soehms closed 2 years ago

soehms commented 4 years ago

Cubic Hecke Algebras are factors of the group algebra of the Artin braid groups, such that the images s_i of the braid generators satisfy a cubic equation:

    s_i^3 = u s_i^2 - v s_i + w

Here u, v, w are elements in an arbitrary integral domain and i is a positive integer less than n, the number of the braid group's strands. By the analogue to the Iwahori Hecke algebras (Sage class) in which the braid generators satisfy a quadratic relation these algebras have been called cubic Hecke algebras. The relations inherited from the braid group are:

    s_i s_{i+1} s_i = s_{i+1} s_i s_{i+1} \mbox{ where } 1\leq i < n-1 \mbox{ and }
    s_i s_j = s_j s_i \mbox{ where } 1 \leq i < j - 1 < n - 1.

If the ring elements u, v, w are taken to be u = v = 0, w = 1 the cubic Hecke algebra specializes to the group algebra of the cubic braid group, which is the factor group of the Artin braid group setting the generators order to be three.

More information on these algebras can be found in "A MAXIMAL CUBIC QUOTIENT OF THE BRAID ALGEBRA" and the references given there.

This ticket implements a class to work with them. It uses data files supplied by Iwan Marin for a basis of the cubic Hecke algebras up to 4 strands and corresponding regular representation matrices. For more than two generators (number of strands larger than three) the implementation is experimental with respect to the following two aspects:

  1. In a technical sense, since this class will cache results in the file system, for example: images of braids to accelerate the calculation of images of longer braid words or products of basis elements.
  2. In a mathematical sense, since the cubic Hecke algebra on more than 4 strands is not equipped with an appropriate basis (not even an algorithm to produce a flat basis according to Iwan Marin's work on the five strand cubic Hecke algebra). At the moment, the "basis" for the algebras on more than 4 strands grows randomly (starting from the one from the data files) according to the users action, without any guarantee that this would lead to a generating set behaving good under specializations.

In general, it would be desirable to replace the dependence on the data files by algorithms implemented in this class. This would make sense as soon as such algorithms have been found which cover the case of five strands, as well.

The code is spread over two existing directories: src/sage/algebras/hecke_algebras/, src/sage/databases and two new sub directories in the former one: base_rings_of_definition and matrix_representations. Both of the new directories contain just one module. Their purpose is to describe the special properties of matrix representations and base rings concerning the cubic Hecke algebra. Thus, in the latter case the corresponding module contains special classes for the most general base ring of the algebra (called the ring of definition) and a corresponding ring for the coefficients of the split irreducible matrix representations.

From the point of view of the algebra the ring of definition must not necessarily contain the roots of the defining cubic equation. Thus, it is defined by \Z[u, v, w, w^(-1)] where the indeterminates are taken from the coefficients of the underlying cubic equation x**3 - u*x**2 + v*x - w. But, concerning the split irreducible representations the roots (in Ivan Marin's papers often denoted a, b, c) are needed, and in addition a third root of unity (for the nine-dimensional representations of the cubic Hecke algebra on 4 strands). Accordingly, this ring is realized as a Laurent polynomial ring in a, b, c over \Z adjoined with a third root of unity. It can be considered as an extension of the ring of definition, more precisely as the splitting algebra of the cubic equation.

These both classes of base rings posses a method create_specialization which allows the user to consider cubic Hecke algebras over other base rings. Thus, the user may choose his own coefficients or roots of the cubic equation as long, as they define a valid ring homomorphism from the corresponding generic base ring.

The module for the matrix representations contains enum classes to specify the type of the representation (left regular, right regular, split irreducible) and to specify a certain irreducible. Furthermore, it implements parent and element classes for a faithful representation (at least in the case of less than 5 strands) in block diagonal structure with irreducibles blocks (in the case of that representation type).

Everything that has to do with the access to the data files and the file cache is implemented in src/sage/databases/cubic_hecke_db.py. The data files are obtained via the pip installable package database_cubic_hecke and the file cache in $HOME/.sage/db/cubic_hecke/. It is created at runtime. Tests concerning the optional package are running as a GitHub workflow which is derived from tox-optional.

CC: @tscrim

Component: algebra

Keywords: cubic Hecke algebra braid group

Author: Sebastian Oehms

Branch/Commit: 8d9deb2

Reviewer: Matthias Koeppe, Travis Scrimshaw

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

soehms commented 4 years ago

tarball for data files

soehms commented 4 years ago
comment:1

Attachment: cubic_hecke_marin-20200513.tar.bz2.gz

soehms commented 4 years ago

Branch: u/soehms/cubic_hecke_algebra_29717

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 4 years ago

Commit: 8c8a522

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 4 years ago

Branch pushed to git repo; I updated commit sha1. New commits:

8c8a52229717: fix upstream_url and some docs
soehms commented 4 years ago

Description changed:

--- 
+++ 
@@ -35,3 +35,12 @@
 The module for the matrix representations contains enum classes to specify the type of the representation (left regular, right regular, split irreducible) and to specify a certain irreducible. Furthermore, it implements parent and element classes for a faithful representation (at least in the case of less than 5 strands) in block diagonal structure with irreducibles blocks (in the case of that representation type).

 Everything that has to do with the access to the data files and the file cache is implemented in `src/sage/databases/cubic_hecke_db.py`. The data files are stored in `local/share/cubic_hecke_marin/` and the file cache in `$HOME/.sage/db/cubic_hecke/`. The former ones are created via the sage-package `cubic_hecke_marin` (from the attached tarball) at build time and the later one at runtime.
+
+Having checked out the ticket for the first time, you have to run
+
+```
+make SAGE_SPKG="sage-spkg -o" cubic_hecke_marin-clean build
+```
+
+in order to have the data files installed.
+
soehms commented 4 years ago

Author: Sebastian Oehms

soehms commented 4 years ago
comment:4

Lets see if the patchbots can do that!

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 4 years ago

Changed commit from 8c8a522 to 637e781

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 4 years ago

Branch pushed to git repo; I updated commit sha1. New commits:

637e78129717: fix optional gap3 doctest
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 4 years ago

Changed commit from 637e781 to 764e5db

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 4 years ago

Branch pushed to git repo; I updated commit sha1. New commits:

ee46039Merge branch 'u/soehms/splitting_algebra' of git://trac.sagemath.org/sage into splitting_algebra_29716
2f0c99c29716 rebasing and pyflakes hints
8c2a4ccMerge branch 'u/soehms/splitting_algebra' of git://trac.sagemath.org/sage into splitting_algebra_29716
5ea214f29716: rebase to 9.2.beta0 and some style fixes
764e5dbMerge branch 'u/soehms/cubic_hecke_algebra_29717' of git://trac.sagemath.org/sage into cubic_hecke_algebra_29717
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 4 years ago

Changed commit from 764e5db to 1d41115

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 4 years ago

Branch pushed to git repo; I updated commit sha1. New commits:

c3fe0daSome mild PEP8 stuff and other formatting stuff for splitting algebras.
0ec9ab1Merge branch 'public/algebras/splitting_algebra-29716' of git://trac.sagemath.org/sage into splitting_algebra_29716
16f7136Merge branch 'u/soehms/cubic_hecke_algebra_29717' of git://trac.sagemath.org/sage into cubic_hecke_algebra_29717
e52eece29716: correct _first_ngens
1d41115Merge branch 'splitting_algebra_29716' into cubic_hecke_algebra_29717
soehms commented 4 years ago
comment:8

I just merged in the fixes of #29716.

soehms commented 4 years ago

Description changed:

--- 
+++ 
@@ -44,3 +44,4 @@

 in order to have the data files installed.

+Traball: https://github.com/sagemath/sage-prod/files/10659408/cubic_hecke_marin-20200513.tar.bz2.gz
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 4 years ago

Branch pushed to git repo; I updated commit sha1. New commits:

785b379Merge branch 'develop' into cubic_hecke_algebra_29717
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 4 years ago

Changed commit from 1d41115 to 785b379

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 4 years ago

Branch pushed to git repo; I updated commit sha1. New commits:

58ceee5Merge branch 'u/soehms/cubic_hecke_algebra_29717' of git://trac.sagemath.org/sage into cubic_hecke_algebra_29717
aea3c5a29717: adaption to ticket 17815
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 4 years ago

Changed commit from 785b379 to aea3c5a

soehms commented 4 years ago
comment:12

I will change the database integration in a similar way as I did in #30352.

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 2 years ago

Changed commit from aea3c5a to 63b8979

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 2 years ago

Branch pushed to git repo; I updated commit sha1. New commits:

430210933584: initial version
e526292clean the mathics.py file
d969e4bMerge branch 'mathics_33581' into mathics_33581_33584
575f9ccMerge branch 'u/soehms/cubic_hecke_algebra_29717' of trac.sagemath.org:sage into cubic_hecke_29717
63b897929717: make smaller algebras independent on optional package
soehms commented 2 years ago
comment:14

After a long break I do some major changes, now:

  1. As announced in the previous comment I change the integration of the database. It now consists of a static part covering the cubic Hecke algebras on at most 3 strand (according to the demonstration cases of the KnotInfo database) and an optional extension serving for the larger algebras. It can be installed via pip. The branch of this ticket represents the stand alone part so that the patchbots will check that everything is o.K. if the optional package is not present. In order to test the integration of the optional package I will open another ticket which will be tested via GitHub actions.

  2. I add another functionality to obtain Markov traces on the cubic Hecke algebras (method formal_markov_trace).

  3. I change the construction of the base ring of definition using localization of multivariate polynomial ring over the integers after this is available now.

  4. A lot of improvements concerning style and documentation.

soehms commented 2 years ago
comment:15

I accidentally merged in #33581 and #33584 but that should not matter!

soehms commented 2 years ago

Changed dependencies from #29432, #29469, #29716 to #33581 #33584

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 2 years ago

Changed commit from 63b8979 to 8b4d286

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 2 years ago

Branch pushed to git repo; I updated commit sha1. New commits:

8b4d286add missing reference
soehms commented 2 years ago
comment:18

I will open another ticket which will be tested via GitHub actions.

This is #33590.

soehms commented 2 years ago

Description changed:

--- 
+++ 
@@ -17,7 +17,7 @@

 More information on these algebras can be found in ["A  MAXIMAL  CUBIC  QUOTIENT  OF  THE  BRAID  ALGEBRA"](http://www.lamfa.u-picardie.fr/marin/arts/GQ.pdf) and the references given there.

-This ticket implements a class to work with them. It uses [data files](http://www.lamfa.u-picardie.fr/marin/representationH4-en.html) supplied by Iwan Marin for a basis of the cubic Hecke algebras up to 4 strands and corresponding regular representation matrices. The implementation is experimental with respect to the following two aspects:
+This ticket implements a class to work with them. It uses [data files](http://www.lamfa.u-picardie.fr/marin/representationH4-en.html) supplied by Iwan Marin for a basis of the cubic Hecke algebras up to 4 strands and corresponding regular representation matrices. For more than two generators (number of strands larger than three) the implementation is experimental with respect to the following two aspects:

 1. In a technical sense, since this class will cache results in the file system, for example: images of braids to accelerate the calculation of images of longer braid words or products of basis elements.
 2. In a mathematical sense, since the cubic Hecke algebra on more than 4 strands is not equipped with an appropriate basis (not even an algorithm to produce a flat basis according to Iwan Marin's work on the five strand cubic Hecke algebra).
@@ -34,14 +34,4 @@

 The module for the matrix representations contains enum classes to specify the type of the representation (left regular, right regular, split irreducible) and to specify a certain irreducible. Furthermore, it implements parent and element classes for a faithful representation (at least in the case of less than 5 strands) in block diagonal structure with irreducibles blocks (in the case of that representation type).

-Everything that has to do with the access to the data files and the file cache is implemented in `src/sage/databases/cubic_hecke_db.py`. The data files are stored in `local/share/cubic_hecke_marin/` and the file cache in `$HOME/.sage/db/cubic_hecke/`. The former ones are created via the sage-package `cubic_hecke_marin` (from the attached tarball) at build time and the later one at runtime.
-
-Having checked out the ticket for the first time, you have to run
-
-```
-make SAGE_SPKG="sage-spkg -o" cubic_hecke_marin-clean build
-```
-
-in order to have the data files installed.
-
-Traball: https://github.com/sagemath/sage-prod/files/10659408/cubic_hecke_marin-20200513.tar.bz2.gz
+Everything that has to do with the access to the data files and the file cache is implemented in `src/sage/databases/cubic_hecke_db.py`. The data files are obtained via the `pip` installable package [database_cubic_hecke https://pypi.org/project/database-cubic-hecke/] and the file cache in `$HOME/.sage/db/cubic_hecke/`. It is created at runtime. The integration of the external database will be treated in the separate ticket #33590.
soehms commented 2 years ago

Description changed:

--- 
+++ 
@@ -28,10 +28,10 @@

 The code is spread over two existing directories: `src/sage/algebras/hecke_algebras/, src/sage/databases` and two new sub directories in the former one: `base_rings_of_definition` and `matrix_representations`. Both of the new directories contain just one module. Their purpose is to describe the special properties of matrix representations and base rings concerning the cubic Hecke algebra. Thus, in the latter case the corresponding module contains special classes for the most general base ring of the algebra (called the *ring of definition*) and a corresponding ring for the coefficients of the split irreducible matrix representations.

-From the point of view of the algebra the ring of definition must not necessarily contain the roots of the defining cubic equation. Thus, it is defined by `\Z[u, v, w, w^(-1)]` where the indeterminates are taken from the coefficients of the underlying cubic equation `x**3 - u*x**2 + v*x - w`. But, concerning the split irreducible representations the roots (in Ivan Marin's papers often denoted `a, b, c`) are needed, and in addition a third root of unity (for the nine-dimensional representations of the cubic Hecke algebra on 4 strands). Accordingly, this ring is realized as a Laurent polynomial ring in `a, b, c` over `\Z` adjoined with a third root of unity. It can be considered as an extension of the ring of definition, more precisely as the [link ticket splitting algebra] of the cubic equation.
+From the point of view of the algebra the ring of definition must not necessarily contain the roots of the defining cubic equation. Thus, it is defined by `\Z[u, v, w, w^(-1)]` where the indeterminates are taken from the coefficients of the underlying cubic equation `x**3 - u*x**2 + v*x - w`. But, concerning the split irreducible representations the roots (in Ivan Marin's papers often denoted `a, b, c`) are needed, and in addition a third root of unity (for the nine-dimensional representations of the cubic Hecke algebra on 4 strands). Accordingly, this ring is realized as a Laurent polynomial ring in `a, b, c` over `\Z` adjoined with a third root of unity. It can be considered as an extension of the ring of definition, more precisely as the [splitting algebra](https://doc.sagemath.org/html/en/reference/algebras/sage/algebras/splitting_algebra.html) of the cubic equation.

 These both classes of base rings posses a method `create_specialization` which allows the user to consider cubic Hecke algebras over other base rings. Thus, the user may choose his own coefficients or roots of the cubic equation as long, as they define a valid ring homomorphism from the corresponding generic base ring.

 The module for the matrix representations contains enum classes to specify the type of the representation (left regular, right regular, split irreducible) and to specify a certain irreducible. Furthermore, it implements parent and element classes for a faithful representation (at least in the case of less than 5 strands) in block diagonal structure with irreducibles blocks (in the case of that representation type).

-Everything that has to do with the access to the data files and the file cache is implemented in `src/sage/databases/cubic_hecke_db.py`. The data files are obtained via the `pip` installable package [database_cubic_hecke https://pypi.org/project/database-cubic-hecke/] and the file cache in `$HOME/.sage/db/cubic_hecke/`. It is created at runtime. The integration of the external database will be treated in the separate ticket #33590.
+Everything that has to do with the access to the data files and the file cache is implemented in `src/sage/databases/cubic_hecke_db.py`. The data files are obtained via the `pip` installable package [database_cubic_hecke](https://pypi.org/project/database-cubic-hecke/) and the file cache in `$HOME/.sage/db/cubic_hecke/`. It is created at runtime. The integration of the external database will be treated in the separate ticket #33590.
tscrim commented 2 years ago
comment:22

Shouldn't the database ticket be a dependency of this one?

I don't think we need the subfolders base_rings_of_definitions and matrix_representations since they effectively only contain one file.

There is one failure reported by the patchbot that needs to be fixed:

sage -t --long --random-seed=191203056443036684306219620613401301840 src/sage/doctest/sources.py
**********************************************************************
File "src/sage/doctest/sources.py", line 782, in sage.doctest.sources.FileDocTestSource._test_enough_doctests
Failed example:
    for path, dirs, files in itertools.chain(os.walk('sage'), os.walk('doc')): # long time
        path = os.path.relpath(path)
        dirs.sort(); files.sort()
        for F in files:
            _, ext = os.path.splitext(F)
            if ext in ('.py', '.pyx', '.pxd', '.pxi', '.sage', '.spyx', '.rst'):
                filename = os.path.join(path, F)
                FDS = FileDocTestSource(filename, DocTestDefaults(long=True, optional=True, force_lib=True))
                FDS._test_enough_doctests(verbose=False)
Expected:
    There are 3 unexpected tests being run in sage/doctest/parsing.py
    There are 1 unexpected tests being run in sage/doctest/reporting.py
Got:
    There are 2 tests in sage/databases/cubic_hecke_db.py that are not being run
    There are 3 unexpected tests being run in sage/doctest/parsing.py
    There are 1 unexpected tests being run in sage/doctest/reporting.py
**********************************************************************

Some Returns -> Return in docstrings.

For TestSuite, you can implement some_elements() with the more interesting elements too.

Perhaps instead of CubicHeckeRingOfDefinition, maybe CubicHeckeUniversalRing? The term "ring of definition" is a bit strange to me, and a bit too generic of a name.

For extension_ring(), I think the extension ring should be the one setting the coercions rather than that method. It feels strange that the method is doing something that the class should "know" how to do.

There are some $ in the docstrings to become `.

Saying optional default: is redundant. I would remove optional.

-    TESTS:
+    TESTS::

         sage: CHA3 = algebras.CubicHecke(3)
         sage: TestSuite(CHA3).run()

Instead of setting self._names = names, I think it is better to pass it up to the CombinatorialFreeModule.__init__.

I will probably have more comments as I go further through the code, but this (still) is a very nice piece of code that I am looking forward to getting merged into Sage.

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 2 years ago

Changed commit from 8b4d286 to 16277c4

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 2 years ago

Branch pushed to git repo; I updated commit sha1. New commits:

16277c429717: fixes according to review
soehms commented 2 years ago
comment:24

Replying to @tscrim:

Shouldn't the database ticket be a dependency of this one?

No, the other way around. I intend to have this branch tested by patchbots (without the database installed).

I don't think we need the subfolders base_rings_of_definitions and matrix_representations since they effectively only contain one file.

So what place do you suggest for these files? I didn't want to have them inside hecke_algebras since they describe no Hecke algebras for themselves. Furthermore, these directories could be used in future, for example if we implement a wrapper around CHEVIE's cyclotomic Hecke algebras. Many of these have specific demands for their generic base rings and all come along with matrix representations.

There is one failure reported by the patchbot that needs to be fixed ...

Fixed.

Some Returns -> Return in docstrings.

Fixed.

For TestSuite, you can implement some_elements() with the more interesting elements too.

At the moment the test for associativity already takes long time with one (interesting) element (for the algebra on four strands up to half an hour, if the filecache is empty). I fear that will increase a lot if there are more elements.

Perhaps instead of CubicHeckeRingOfDefinition, maybe CubicHeckeUniversalRing? The term "ring of definition" is a bit strange to me, and a bit too generic of a name.

I asked Ivan Marin for a suggestion for that name (my suggestion was the same as yours). Here his argumentation (dating from July 2019) that convinced me:

Other terminology which are compatible with algebraic geometry and which I sometimes use are "ring of definition", "defining rings". I prefer these terms to "universal base rings", because in that case the problem is : universal among what ? Every possible specialization ? Kind of chicken and egg problem...

But you are right, this is not commonly used and to be honest: at the moment I don't see his chicken and egg problem any more.

For extension_ring(), I think the extension ring should be the one setting the coercions rather than that method. It feels strange that the method is doing something that the class should "know" how to do.

I don't understand your point here. Should the method extension_ring return a coercion map rather than a ring? Or do you mean that it is redundant?

There are some $ in the docstrings to become `.

Fixed.

Saying optional default: is redundant. I would remove optional.

Fixed.

-    TESTS:
+    TESTS::

         sage: CHA3 = algebras.CubicHecke(3)
         sage: TestSuite(CHA3).run()

Fixed.

Instead of setting self._names = names, I think it is better to pass it up to the CombinatorialFreeModule.__init__.

Fixed.

I will probably have more comments as I go further through the code, but this (still) is a very nice piece of code that I am looking forward to getting merged into Sage.

Many thanks!

tscrim commented 2 years ago
comment:25

Replying to @soehms:

Replying to @tscrim:

Shouldn't the database ticket be a dependency of this one?

No, the other way around. I intend to have this branch tested by patchbots (without the database installed).

The patchbots test things without installing optional packages (unless their owners have installed them, but generally that is not the case). I would think logically you want to have this tested with the package, which can be merged without this (albeit with limited use within Sage by default). Well, it's not anything important as they will likely be merged at the same time.

I don't think we need the subfolders base_rings_of_definitions and matrix_representations since they effectively only contain one file.

So what place do you suggest for these files? I didn't want to have them inside hecke_algebras since they describe no Hecke algebras for themselves.

I would place them in there because they are used to create the Hecke algebras in question. I don't see what difference it makes for it being a subfolder as it just adds more complexity to the file hierarchy.

Furthermore, these directories could be used in future, for example if we implement a wrapper around CHEVIE's cyclotomic Hecke algebras. Many of these have specific demands for their generic base rings and all come along with matrix representations.

If it makes sense to do this because the files in that directory become sufficiently complicated, then we can do it then. However, where we are now, it makes things more cumbersome, not less.

Perhaps instead of CubicHeckeRingOfDefinition, maybe CubicHeckeUniversalRing? The term "ring of definition" is a bit strange to me, and a bit too generic of a name.

I asked Ivan Marin for a suggestion for that name (my suggestion was the same as yours). Here his argumentation (dating from July 2019) that convinced me:

Other terminology which are compatible with algebraic geometry and which I sometimes use are "ring of definition", "defining rings". I prefer these terms to "universal base rings", because in that case the problem is : universal among what ? Every possible specialization ? Kind of chicken and egg problem...

But you are right, this is not commonly used and to be honest: at the moment I don't see his chicken and egg problem any more.

Something like this perhaps:

Q1: Universal wrt to what?

A1: A Hecke algebra with a universal base ring

Q2: What is its universal base ring?

A2: See Q1.

Personally I think we get a more explicit chicken-and-egg-problem with "ring of definition." I also would not know how to make sense of this ring from the name. Well, it isn't anything that needs to be changed.

For extension_ring(), I think the extension ring should be the one setting the coercions rather than that method. It feels strange that the method is doing something that the class should "know" how to do.

I don't understand your point here. Should the method extension_ring return a coercion map rather than a ring? Or do you mean that it is redundant?

No, it is about who is saying there is a coercion from B -> E. Is it in B.e() or in E? The standard way is E._coerce_map_from_(), which can also be done from a particular morphism. Additionally, since E should be a UniqueRepresentation, there should not be any caching in e() (which can and usually does) create memory leaks. E should allow itself to be removed from memory when there is no way to obtain it from the current references. Basically it is about moving the line:

ExtensionRing.register_conversion(embedding_into_extension_ring)

and the other related code necessary to do this.

I will probably have more comments as I go further through the code, but this (still) is a very nice piece of code that I am looking forward to getting merged into Sage.

Many thanks!

Sorry for the limited reply. I am finishing up my day. I should be able to get through more of this next week.

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 2 years ago

Branch pushed to git repo; I updated commit sha1. New commits:

42332e2Merge branch 'u/soehms/cubic_hecke_algebra_29717' of trac.sagemath.org:sage into cubic_hecke_29717
e270a7c33590 initial version
211f72633590 correct dependencies
0c66a8833590 add conway_polynomials to dependencies
1bc565dMerge branch 'cubic_hecke_29717' into database_cubic_hecke_33590
86b3f9533590: take care of CHEVIE
f197babMerge branch 'u/soehms/database_cubic_hecke_33590' of trac.sagemath.org:sage into cubic_hecke_29717
cc3c11329717: implement `_coerce_map_from_` for extension ring
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 2 years ago

Changed commit from 16277c4 to cc3c113

soehms commented 2 years ago
comment:27

Replying to @tscrim:

Replying to @soehms:

Replying to @tscrim:

Shouldn't the database ticket be a dependency of this one?

No, the other way around. I intend to have this branch tested by patchbots (without the database installed).

The patchbots test things without installing optional packages (unless their owners have installed them, but generally that is not the case). I would think logically you want to have this tested with the package, which can be merged without this (albeit with limited use within Sage by default). Well, it's not anything important as they will likely be merged at the same time.

You are right, I expected that the patchbots will not run on #33590 since it has modified files under build/pkgs. I expected a purple cube instead of the patchbot badged. But astonishingly the patchbot run on that branch after I set needs review as well. So there is no need for that ticket any more. I merge it into this and will close it as duplicate.

I don't think we need the subfolders base_rings_of_definitions and matrix_representations since they effectively only contain one file.

So what place do you suggest for these files? I didn't want to have them inside hecke_algebras since they describe no Hecke algebras for themselves.

I would place them in there because they are used to create the Hecke algebras in question. I don't see what difference it makes for it being a subfolder as it just adds more complexity to the file hierarchy.

Furthermore, these directories could be used in future, for example if we implement a wrapper around CHEVIE's cyclotomic Hecke algebras. Many of these have specific demands for their generic base rings and all come along with matrix representations.

If it makes sense to do this because the files in that directory become sufficiently complicated, then we can do it then. However, where we are now, it makes things more cumbersome, not less.

Convinced! I will do that on a next step.

Perhaps instead of CubicHeckeRingOfDefinition, maybe CubicHeckeUniversalRing? The term "ring of definition" is a bit strange to me, and a bit too generic of a name.

I asked Ivan Marin for a suggestion for that name (my suggestion was the same as yours). Here his argumentation (dating from July 2019) that convinced me:

Other terminology which are compatible with algebraic geometry and which I sometimes use are "ring of definition", "defining rings". I prefer these terms to "universal base rings", because in that case the problem is : universal among what ? Every possible specialization ? Kind of chicken and egg problem...

But you are right, this is not commonly used and to be honest: at the moment I don't see his chicken and egg problem any more.

Something like this perhaps:

Q1: Universal wrt to what?

A1: A Hecke algebra with a universal base ring

Q2: What is its universal base ring?

A2: See Q1.

Yes, that's the point!

Personally I think we get a more explicit chicken-and-egg-problem with "ring of definition." I also would not know how to make sense of this ring from the name. Well, it isn't anything that needs to be changed.

I add an according explanation to the docstring.

For extension_ring(), I think the extension ring should be the one setting the coercions rather than that method. It feels strange that the method is doing something that the class should "know" how to do.

I don't understand your point here. Should the method extension_ring return a coercion map rather than a ring? Or do you mean that it is redundant?

No, it is about who is saying there is a coercion from B -> E. Is it in B.e() or in E? The standard way is E._coerce_map_from_(), which can also be done from a particular morphism. Additionally, since E should be a UniqueRepresentation, there should not be any caching in e() (which can and usually does) create memory leaks. E should allow itself to be removed from memory when there is no way to obtain it from the current references. Basically it is about moving the line:

ExtensionRing.register_conversion(embedding_into_extension_ring)

and the other related code necessary to do this.

I misunderstood that because I was looking at the wrong extension_ring method (that of CubicHeckeAlgebra). I have fixed this, now.

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 2 years ago

Changed commit from cc3c113 to bfcf191

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 2 years ago

Branch pushed to git repo; I updated commit sha1. New commits:

ef254a1Merge branch 'u/soehms/cubic_hecke_algebra_29717' of trac.sagemath.org:sage into cubic_hecke_29717
fa544e3Merge branch 'u/soehms/cubic_hecke_algebra_29717' of trac.sagemath.org:sage into cubic_hecke_29717
bfcf19129717: reorganize directories
soehms commented 2 years ago

Changed dependencies from #33581 #33584 to none

soehms commented 2 years ago
comment:30

The current patchbot failures seem to be caused by a leftover file cache. I will see how I can ensure these to be cleared.

The first workflow run including all optional tests (database_cubic_hecke + gap3) are fine (the build failures on some platforms seem to be unrelated). For example gentoo python 3.10:

Using --optional=database_cubic_hecke,gap3,sage
Features to be detected: 4ti2,benzene,bliss,buckygen,conway_polynomials,csdp,database_cremona_ellcurve,database_cremona_mini_ellcurve,database_cubic_hecke,database_jones_numfield,database_knotinfo,dvipng,graphviz,imagemagick,jupymake,kenzo,latte_int,lrslib,mcqd,meataxe,nauty,palp,pandoc,pdf2svg,pdftocairo,plantri,polytopes_db,polytopes_db_4d,pynormaliz,python_igraph,rubiks,sage.combinat,sage.geometry.polyhedron,sage.graphs,sage.groups,sage.plot,sage.rings.number_field,sage.rings.padics,sage.rings.real_double,sage.symbolic,sage_numerical_backends_coin,sagemath_doc_html,sphinx,tdlib
Sorting sources by runtime so that slower doctests are run first....
Doctesting 9 files using 3 threads.
sage -t --long --random-seed=104303597285919165653703494900311160938 algebras/hecke_algebras/__init__.py
    [0 tests, 0.00 s]
sage -t --long --random-seed=104303597285919165653703494900311160938 algebras/hecke_algebras/all.py
    [0 tests, 0.00 s]
sage -t --long --random-seed=104303597285919165653703494900311160938 algebras/hecke_algebras/base_rings_of_definition/__init__.py
    [0 tests, 0.00 s]
sage -t --long --random-seed=104303597285919165653703494900311160938 algebras/hecke_algebras/ariki_koike_algebra.py
    [263 tests, 16.69 s]
sage -t --long --random-seed=104303597285919165653703494900311160938 algebras/hecke_algebras/matrix_representations/__init__.py
    [0 tests, 0.00 s]
sage -t --long --random-seed=104303597285919165653703494900311160938 algebras/hecke_algebras/base_rings_of_definition/cubic_hecke_base_ring.py
    [225 tests, 13.29 s]
sage -t --long --random-seed=104303597285919165653703494900311160938 algebras/hecke_algebras/matrix_representations/cubic_hecke_matrix_rep.py
    [131 tests, 7.40 s]
sage -t --long --random-seed=104303597285919165653703494900311160938 databases/cubic_hecke_db.py
    [171 tests, 8.86 s]
sage -t --long --random-seed=104303597285919165653703494900311160938 algebras/hecke_algebras/cubic_hecke_algebra.py
    [373 tests, 218.98 s]
----------------------------------------------------------------------
All tests passed!
----------------------------------------------------------------------
Total time for all tests: 229.3 seconds
    cpu time: 237.0 seconds
    cumulative wall time: 265.2 seconds
Features detected for doctesting: 
Pytest is not installed, skip checking tests that rely on it.
soehms commented 2 years ago

Description changed:

--- 
+++ 
@@ -34,4 +34,5 @@

 The module for the matrix representations contains enum classes to specify the type of the representation (left regular, right regular, split irreducible) and to specify a certain irreducible. Furthermore, it implements parent and element classes for a faithful representation (at least in the case of less than 5 strands) in block diagonal structure with irreducibles blocks (in the case of that representation type).

-Everything that has to do with the access to the data files and the file cache is implemented in `src/sage/databases/cubic_hecke_db.py`. The data files are obtained via the `pip` installable package [database_cubic_hecke](https://pypi.org/project/database-cubic-hecke/) and the file cache in `$HOME/.sage/db/cubic_hecke/`. It is created at runtime. The integration of the external database will be treated in the separate ticket #33590.
+Everything that has to do with the access to the data files and the file cache is implemented in `src/sage/databases/cubic_hecke_db.py`. The data files are obtained via the `pip` installable package [database_cubic_hecke](https://pypi.org/project/database-cubic-hecke/) and the file cache in `$HOME/.sage/db/cubic_hecke/`. It is created at runtime. Tests concerning the optional package are running as a [GitHub workflow](https://github.com/soehms/sage/actions/workflows/tox-database_cubic_hecke.yml) which is derived from `tox-optional`.
+
mkoeppe commented 2 years ago

Reviewer: Matthias Koeppe, ...

mkoeppe commented 2 years ago
comment:31

The new package in build/pkgs/database_cubic_hecke and the additions in sage.features look OK. I would only suggest to shorten the title in SPKG.rst a bit. I can't comment on the other parts of the ticket.

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 2 years ago

Changed commit from bfcf191 to 84619e7

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 2 years ago

Branch pushed to git repo; I updated commit sha1. New commits:

84619e729717: fix file-cache issue
soehms commented 2 years ago
comment:33

The former commit removes the last dependency on the Sage library from the file-cache (it now consists of nothing else than dictionaries, lists, tuples, integers and strings).

The expected behavior for the patchbot which did the two jobs for this ticket would be to raise the new exception. All other patchbots should be green. Is there a way to clean SAGE_DOT files on that special patchbot?

The GitHub jobs running on the commit with the dropped directories look good. But note, that not all jobs did test for gap3 (linux-mint-18 did fedora-36 did not).

soehms commented 2 years ago
comment:34

Replying to @mkoeppe:

The new package in build/pkgs/database_cubic_hecke and the additions in sage.features look OK. I would only suggest to shorten the title in SPKG.rst a bit.

This is done, now. Thanks for having a look at it!