Sage: Ticket #18700: Have GroupAlgebra(Q, R) and G.algebra(R) return the same standard class for group algebras
https://trac.sagemath.org/ticket/18700
<p>
Consider the following situation:
</p>
<pre class="wiki">sage: P = RootSystem(['A',2,1]).weight_lattice()
sage: W = RootSystem(['A',2,1]).weight_space()
sage: W.has_coerce_map_from(P)
True
sage: PA = P.algebra(QQ)
sage: WA = W.algebra(QQ)
sage: WA.has_coerce_map_from(PA)
False
</pre><p>
The last line should be <code>True</code>. This comes from the fact that <code>GroupAlgebra</code> implements some extra coercion logic and <code>G.algebra(R)</code> does not return an instance of this class.
</p>
<p>
To remedy this, we lift a good portion of the <code>GroupAlgebra</code> code to the appropriate categories, where we impose the restriction that a <code>(Semi)Group.Algebra</code> must have a basis indexed by elements of the (semi)group, and have all group algebras redirect to the now (lightweight) class <code>GroupAlgebra_class</code> in order to implement coercions.
</p>
en-usSagehttps://trac.sagemath.org/chrome/site/logo_sagemath_trac.png
https://trac.sagemath.org/ticket/18700
Trac 1.1.6tscrimSun, 14 Jun 2015 15:49:55 GMT
https://trac.sagemath.org/ticket/18700#comment:1
https://trac.sagemath.org/ticket/18700#comment:1
<p>
Actually, <code>GroupAlgebra</code> already has this behavior. However <code>algebra()</code> does not return the group algebra by default. I think we need 2 changes:
</p>
<p>
1 - <code>algebra()</code> checks if <code>self</code> is a group (it already checks for inclusion in finite groups).
2 - We need a subclass of <code>CombinatorialFreeModule</code> which inherits the coercions in general, say a <code>SemigroupAlgebra</code>.
</p>
<p>
I think both changes are relatively easy since for the second we can canabalize a good part of the <code>GroupAlgebra</code> code. However if 2 is contraversal/harder-than-I-thought, then we should just do 1 here as we need this for <a class="closed ticket" href="https://trac.sagemath.org/ticket/18453" title="defect: Infinite affine crystals should use extended weight lattice (closed: fixed)">#18453</a> and do a followup for 2.
</p>
TickettscrimSun, 14 Jun 2015 15:54:20 GMT
https://trac.sagemath.org/ticket/18700#comment:2
https://trac.sagemath.org/ticket/18700#comment:2
<p>
Also as a little side bug:
</p>
<pre class="wiki">sage: P.algebra(QQ)
Group algebra of the Weight lattice of the Root system of type ['A', 2, 1] over Rational Field
sage: _.category()
Category of root lattice realizations over Integer Rin algebras over Rational Field
</pre><p>
Notice the missing <code>g</code>, same thing for <code>W</code> (except there it's <code>...Fiel algebras over...</code>).
</p>
TickettscrimMon, 01 May 2017 23:12:05 GMTstatus, cc, milestone changed; commit, branch set
https://trac.sagemath.org/ticket/18700#comment:3
https://trac.sagemath.org/ticket/18700#comment:3
<ul>
<li><strong>status</strong>
changed from <em>new</em> to <em>needs_review</em>
</li>
<li><strong>commit</strong>
set to <em>5e9420816a245339735f51d0958b06eed509cbd2</em>
</li>
<li><strong>cc</strong>
<em>chapoton</em> added
</li>
<li><strong>branch</strong>
set to <em>public/groups/standardize_group_algebras-18700</em>
</li>
<li><strong>milestone</strong>
changed from <em>sage-6.8</em> to <em>sage-8.0</em>
</li>
</ul>
<p>
This goes through and redirects group algebra construction through <code>GroupAlgebra</code>, as well as cleans up how the categories are constructed.
</p>
<hr />
<p>
New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="https://git.sagemath.org/sage.git/commit?id=5e9420816a245339735f51d0958b06eed509cbd2"><span class="icon"></span>5e94208</a></td><td><code>Standardizing group algebras through GroupAlgebra.</code>
</td></tr></table>
Ticketbsalisbury1Thu, 04 May 2017 00:07:25 GMT
https://trac.sagemath.org/ticket/18700#comment:4
https://trac.sagemath.org/ticket/18700#comment:4
<p>
Hi Travis,
</p>
<p>
There are doctest failures in a lot of difference places. For example:
</p>
<pre class="wiki">----------------------------------------------------------------------
sage -t --long src/sage/combinat/root_system/non_symmetric_macdonald_polynomials.py # 94 doctests failed
sage -t --long src/sage/plot/plot.py # 1 doctest failed
sage -t --long src/sage/plot/graphics.py # 2 doctests failed
sage -t --long src/sage/misc/sagedoc.py # 3 doctests failed
sage -t --long src/sage/combinat/crystals/littelmann_path.py # 6 doctests failed
sage -t --long src/sage/combinat/rigged_configurations/rigged_configurations.py # 13 doctests failed
sage -t --long src/sage/combinat/root_system/root_lattice_realization_algebras.py # 89 doctests failed
sage -t --long src/sage/combinat/posets/posets.py # 1 doctest failed
sage -t --long src/sage/combinat/root_system/hecke_algebra_representation.py # 33 doctests failed
sage -t --long src/sage/categories/finite_dimensional_algebras_with_basis.py # 2 doctests failed
sage -t --long src/sage/combinat/crystals/tensor_product.py # 5 doctests failed
sage -t --long src/sage/repl/rich_output/pretty_print.py # 1 doctest failed
sage -t --long src/doc/en/thematic_tutorials/lie/affine_finite_crystals.rst # 1 doctest failed
sage -t --long src/sage/modules/with_basis/representation.py # 13 doctests failed
sage -t --long src/sage/categories/sets_cat.py # 4 doctests failed
sage -t --long src/sage/algebras/lie_algebras/lie_algebra.py # 2 doctests failed
sage -t --long src/sage/categories/semisimple_algebras.py # 1 doctest failed
----------------------------------------------------------------------
</pre><p>
So many, in fact, that I'm wondering if I did something wrong! Did you get this same failures before you pushed the latest merge to the branch?
</p>
TickettscrimThu, 04 May 2017 01:07:16 GMT
https://trac.sagemath.org/ticket/18700#comment:5
https://trac.sagemath.org/ticket/18700#comment:5
<p>
Those are probably trivial doctest failures because of the change in the how the group algebra elements are printed and I didn't test as well as I thought. I will fix them shortly.
</p>
TicketgitThu, 04 May 2017 03:36:01 GMTcommit changed
https://trac.sagemath.org/ticket/18700#comment:6
https://trac.sagemath.org/ticket/18700#comment:6
<ul>
<li><strong>commit</strong>
changed from <em>5e9420816a245339735f51d0958b06eed509cbd2</em> to <em>39800cc553693c20e460f16f009a185f2baeebdf</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1. New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="https://git.sagemath.org/sage.git/commit/?id=39800cc553693c20e460f16f009a185f2baeebdf"><span class="icon"></span>39800cc</a></td><td><code>Fixing doctests and some conversion behavior changes.</code>
</td></tr></table>
TickettscrimThu, 04 May 2017 03:48:30 GMTsummary changed
https://trac.sagemath.org/ticket/18700#comment:7
https://trac.sagemath.org/ticket/18700#comment:7
<ul>
<li><strong>summary</strong>
changed from <em>Group algebras should have a coercion inherited from coercions of the underlying groups</em> to <em>Use GroupAlgebra as the standard class for group algebras</em>
</li>
</ul>
<p>
This should fix all of the doctest failures that were introduced on this ticket. I did a little bit of a hack for algebras of a module with a basis to wrap them as <code>B[x + y]</code>. Some of the other doctest failures I get on <code>develop</code> (in particular, the failure in <code>posets.py</code>) or cannot reproduce (e.g., the ones in plot).
</p>
Ticketbsalisbury1Thu, 04 May 2017 22:53:32 GMT
https://trac.sagemath.org/ticket/18700#comment:8
https://trac.sagemath.org/ticket/18700#comment:8
<p>
Still getting a doctest failure...
</p>
<pre class="wiki">sage -t src/sage/algebras/group_algebra.py
**********************************************************************
File "src/sage/algebras/group_algebra.py", line 696, in sage.algebras.group_algebra.GroupAlgebra._element_constructor_
Failed example:
OG(2)
Exception raised:
Traceback (most recent call last):
File "/Users/Ben/sage-git/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 509, in _run
self.compile_and_execute(example, compiler, test.globs)
File "/Users/Ben/sage-git/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 872, in compile_and_execute
exec(compiled, globs)
File "<doctest sage.algebras.group_algebra.GroupAlgebra._element_constructor_[11]>", line 1, in <module>
OG(Integer(2))
File "sage/structure/parent.pyx", line 941, in sage.structure.parent.Parent.__call__ (/Users/Ben/sage-git/src/build/cythonized/sage/structure/parent.c:9839)
return mor._call_(x)
File "sage/structure/coerce_maps.pyx", line 110, in sage.structure.coerce_maps.DefaultConvertMap_unique._call_ (/Users/Ben/sage-git/src/build/cythonized/sage/structure/coerce_maps.c:4895)
raise
File "sage/structure/coerce_maps.pyx", line 105, in sage.structure.coerce_maps.DefaultConvertMap_unique._call_ (/Users/Ben/sage-git/src/build/cythonized/sage/structure/coerce_maps.c:4762)
return C._element_constructor(x)
File "/Users/Ben/sage-git/local/lib/python2.7/site-packages/sage/algebras/group_algebra.py", line 720, in _element_constructor_
raise TypeError("do not know how to make x (= %s) an element of %s"%(x, self))
TypeError: do not know how to make x (= 2) an element of Group algebra of General Linear Group of degree 2 over Finite Field of size 7 over Order in Number Field in sqrt5 with defining polynomial x^2 - 5
**********************************************************************
1 item had failures:
1 of 16 in sage.algebras.group_algebra.GroupAlgebra._element_constructor_
[132 tests, 1 failure, 6.13 s]
</pre>
TicketgitThu, 04 May 2017 22:59:12 GMTcommit changed
https://trac.sagemath.org/ticket/18700#comment:9
https://trac.sagemath.org/ticket/18700#comment:9
<ul>
<li><strong>commit</strong>
changed from <em>39800cc553693c20e460f16f009a185f2baeebdf</em> to <em>2ef2208407ec7edfbe03a8c9980741f25fc2db2a</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1. New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="https://git.sagemath.org/sage.git/commit/?id=2ef2208407ec7edfbe03a8c9980741f25fc2db2a"><span class="icon"></span>2ef2208</a></td><td><code>We have a unit, so we can use that in coercion.</code>
</td></tr></table>
TickettscrimThu, 04 May 2017 22:59:31 GMT
https://trac.sagemath.org/ticket/18700#comment:10
https://trac.sagemath.org/ticket/18700#comment:10
<p>
This was me being stupid. Fixed.
</p>
Ticketbsalisbury1Thu, 04 May 2017 23:50:27 GMTstatus changed; reviewer set
https://trac.sagemath.org/ticket/18700#comment:11
https://trac.sagemath.org/ticket/18700#comment:11
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>positive_review</em>
</li>
<li><strong>reviewer</strong>
set to <em>Ben Salisbury</em>
</li>
</ul>
<p>
All tests passed on my machine, the html doc builds, and so does the pdf doc. Moreover:
</p>
<pre class="wiki">sage: P = RootSystem(['A',2,1]).weight_lattice()
sage: W = RootSystem(['A',2,1]).weight_space()
sage: W.has_coerce_map_from(P)
True
sage: PA = P.algebra(QQ)
sage: WA = W.algebra(QQ)
sage: WA.has_coerce_map_from(PA)
True
</pre>
TicketnthieryFri, 05 May 2017 12:54:01 GMTstatus changed
https://trac.sagemath.org/ticket/18700#comment:12
https://trac.sagemath.org/ticket/18700#comment:12
<ul>
<li><strong>status</strong>
changed from <em>positive_review</em> to <em>needs_review</em>
</li>
</ul>
<p>
Hi,
</p>
<p>
Sorry for the interuption; I'd like to have a look at this one. If you don't hear from me in the next day, please set it back to positive review.
</p>
TicketnthieryFri, 05 May 2017 13:24:12 GMT
https://trac.sagemath.org/ticket/18700#comment:13
https://trac.sagemath.org/ticket/18700#comment:13
<p>
Merging <code>GroupAlgebra(G)</code> and <code>G.algebra()</code> has been on the pile for such a long while. Thank you so much for working on it!
</p>
<p>
So far I have always wanted to make <code>GroupAlgebra</code> as thin as possible by lifting stuff to the categories, and if at all possible get rid of it (up to possibly keeping a trivial redirect <code>GroupAlgebra(G)</code> -> <code>G.algebra()</code> for user convenience and backward compatibility). This is much more generic/flexible: for example, the feature at hand (lifting coercions from XXX to XXX algebras) is not specific to multiplicative groups, right? We would want to have the same feature for semigroups or additive groups, right?
</p>
<p>
The current implementation of this ticket tends to go the other way around.
</p>
<p>
So, the core question is: is there any feature in <code>GroupAlgebra</code> that really requires a concrete parent? Or could we just lift everything to categories.
</p>
<p>
Doing it in two steps, first this ticket as is, and then a latter ticket has the inconvenient to change lots of outputs while we may actually want to standardize the outputs the other way around.
</p>
<p>
What do you think?
</p>
<p>
Cheers,
</p>
TicketnthieryFri, 05 May 2017 13:24:38 GMTstatus changed
https://trac.sagemath.org/ticket/18700#comment:14
https://trac.sagemath.org/ticket/18700#comment:14
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>needs_info</em>
</li>
</ul>
TickettscrimFri, 05 May 2017 14:21:39 GMT
https://trac.sagemath.org/ticket/18700#comment:15
https://trac.sagemath.org/ticket/18700#comment:15
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/18700#comment:13" title="Comment 13">nthiery</a>:
</p>
<blockquote class="citation">
<p>
So far I have always wanted to make <code>GroupAlgebra</code> as thin as possible by lifting stuff to the categories, and if at all possible get rid of it (up to possibly keeping a trivial redirect <code>GroupAlgebra(G)</code> -> <code>G.algebra()</code> for user convenience and backward compatibility). This is much more generic/flexible: for example, the feature at hand (lifting coercions from XXX to XXX algebras) is not specific to multiplicative groups, right? We would want to have the same feature for semigroups or additive groups, right?
</p>
</blockquote>
<p>
We can definitely lift many methods up to the category. Some of them are even unnecessary, e.g., <code>is_finite</code> is handled by the <code>FiniteDimensionalModules.extra_super_categories</code> (or something like that) by adding the <code>Finite</code> axiom.
</p>
<blockquote class="citation">
<p>
The current implementation of this ticket tends to go the other way around.
</p>
</blockquote>
<p>
I would say it is mostly neutral to this. The part that goes the other way is moving the addition of the semisimple axiom check, but I feel the code in <code>Sets.ParentMethods.algebra</code> is largely a hack and probably should be distributed to the appropriate subcategories. For now, this is a common point of intersection that is cleaner IMO.
</p>
<blockquote class="citation">
<p>
So, the core question is: is there any feature in <code>GroupAlgebra</code> that really requires a concrete parent? Or could we just lift everything to categories.
</p>
</blockquote>
<p>
While it may not be quite what you mean, I'm considering CFM not as a concrete parent below.
</p>
<p>
One thing that could be important is the coercion into the basis index set. This would be possible to do in CFM, but it probably is not a feature we want in general because it might cause further ambiguities (e.g., a <strong>Q</strong>-module whose basis index set is <strong>Z</strong>). IMO, we definitely want this for group algebras.
</p>
<p>
The other coercion that I think we absolutely need a concrete parent is for R[G] -> R[H] when there is a coercion G -> H. Granted, this is something that could be done at a functor level, but I don't think this is possible currently.
</p>
<blockquote class="citation">
<p>
Doing it in two steps, first this ticket as is, and then a latter ticket has the inconvenient to change lots of outputs while we may actually want to standardize the outputs the other way around.
</p>
</blockquote>
<p>
The biggest thing is the default output for group elements, which is something that we can (maybe should) decide here. If we decide to keep this output but go to a generic CFM with category implementation, then we can pass the appropriate parameters during the construction. So the output is an independent question.
</p>
<p>
I'm in favor of the output of <code>GroupAlgebra</code>, and in an ideal world, it would check the repr atomicness of the basis keys. However, this is not done properly for a number of groups and would likely be ineffective. Actually, we probably should lift <code>element_ascii_art</code> of <code>_repr_options</code> from the basis keys in CFM (separate ticket).
</p>
TicketnthieryFri, 05 May 2017 16:07:42 GMT
https://trac.sagemath.org/ticket/18700#comment:16
https://trac.sagemath.org/ticket/18700#comment:16
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/18700#comment:15" title="Comment 15">tscrim</a>:
</p>
<blockquote class="citation">
<p>
We can definitely lift many methods up to the category. Some of them
are even unnecessary, e.g., <code>is_finite</code> is handled by the
<code>FiniteDimensionalModules.extra_super_categories</code> (or something like
that) by adding the <code>Finite</code> axiom.
</p>
</blockquote>
<p>
Ok; that was my impression too. Glad this is confirmed by someone who
actually manipulated the code recently :-)
</p>
<blockquote class="citation">
<p>
I would say it is mostly neutral to this.
</p>
</blockquote>
<p>
I mostly meant: it increases the usage of <a class="missing wiki">GroupAlgebra?</a> rather than
decreasing it.
</p>
<blockquote class="citation">
<p>
The part that goes the other way is moving the addition of the
semisimple axiom check, but I feel the code in
<code>Sets.ParentMethods.algebra</code> is largely a hack and probably should
be distributed to the appropriate subcategories.
</p>
</blockquote>
<p>
Agreed! For this use case and others, I have been wishing for a hook
in the category infrastructure enabling a category C to inspect parent
upon their initalization and provide additional categories they should
be endowed with.
</p>
<p>
Maybe we don't actually need new infrastructure, and could just use
<span class="underline">init_extra</span> and _refine_category():
</p>
<blockquote>
<p>
def Groups.Finite.Algebras.<a class="missing wiki">ParentMethods?</a>.<span class="underline">init_extra</span>(self)
</p>
<blockquote>
<p>
G = ...
R = ...
if ... non modular ...:
</p>
<blockquote>
<p>
self._refine_category(...<a class="missing wiki">SemiSimple?</a>())
</p>
</blockquote>
</blockquote>
</blockquote>
<p>
Bearing that in mind, we may not want to move the code to an
intermediate spot when it's plausibly going to move again later.
</p>
<blockquote class="citation">
<p>
While it may not be quite what you mean
</p>
</blockquote>
<p>
Sorry for the ambiguity. Let me try to clarify:
</p>
<ul><li>Indeed we don't want a coercion F -> R.F for a general CFM R.F.
Most likely we don't want a conversion either.
</li></ul><ul><li>For a group algebra, the conversion G -> R.G indeed sounds
reasonable. I am unsure at this stage about a coercion; in such
doubt my usual approach is to not put a coercion in such a case;
later it's harder to remove a coercion than add one.
</li></ul><blockquote>
<p>
We don't need to have a concrete class <a class="missing wiki">GroupAlgebra?</a> for this. Such a
coercion could be declared in
<code>Groups.Algebra.ParentMethods.__init__extra</code>. Presumably we could
generalize it to <code>Magmas.Unital.ParentMethods.__init_extra</code> and its
additive equivalent.
</p>
</blockquote>
<ul><li>The lifting of a coercion G-> H to R[G] -> R[H] for group algebras
looks reasonable indeed. Wouldn't be enough to move the
<code>construction</code> method to <code>Groups.ParentMethods</code>, and update the
functor to call <code>G.algebra(R)</code> rather than <code>GroupAlgebra(G,R)</code>?
(I haven't tried).
</li></ul><blockquote>
<p>
This could actually be lifted to <code>Magmas.ParentMethods</code> and its
additive counterpart.
</p>
</blockquote>
<p>
With that, the concrete class "<a class="missing wiki">GroupAlgebra?</a>" could be replaced by a
simple alias <code>GroupAlgebra(G,R)</code> -> <code>G.algebra(R)</code>, right?
</p>
<blockquote class="citation">
<p>
The biggest thing is the default output for group elements, which is
something that we can (maybe should) decide here.
</p>
</blockquote>
<p>
+1
</p>
<blockquote class="citation">
<p>
If we decide to keep this output but go to a generic CFM with
category implementation, then we can pass the appropriate parameters
during the construction. So the output is an independent question.
</p>
</blockquote>
<p>
+1. This could be done in Magmas.Algebras.<span class="underline">init</span>extra_ with set_options.
Or we could overwrite the _repr methods in <a class="missing wiki">MagmaAlgebra?</a>.<a class="missing wiki">ElementMethods?</a>.
</p>
<blockquote class="citation">
<p>
I'm in favor of the output of <code>GroupAlgebra</code>
</p>
</blockquote>
<p>
I agree that the repr of <a class="missing wiki">GroupAlgebra?</a> is nicely concise and close to
usual math notations; so that could be a good option.
</p>
<p>
Opinions anyone? Are there are cases where there could be ambiguity
and we would want something slightly more verbose?
</p>
<blockquote class="citation">
<p>
and in an ideal world, it would check the repr atomicness of the
basis keys.
</p>
</blockquote>
<p>
Not sure what you mean, but that's fine :-)
</p>
TickettscrimFri, 05 May 2017 18:38:42 GMT
https://trac.sagemath.org/ticket/18700#comment:17
https://trac.sagemath.org/ticket/18700#comment:17
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/18700#comment:16" title="Comment 16">nthiery</a>:
</p>
<blockquote class="citation">
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/18700#comment:15" title="Comment 15">tscrim</a>:
</p>
<blockquote class="citation">
<p>
I would say it is mostly neutral to this.
</p>
</blockquote>
<p>
I mostly meant: it increases the usage of <a class="missing wiki">GroupAlgebra?</a> rather than
decreasing it.
</p>
</blockquote>
<p>
Well, I am not sure we are going to be able to escape the coercion issue. At least for now, because <code>GroupAlgebra</code> will be a small extra layer over CFM and there could be other thorny issues with getting the functionality we'd want, I think we should utilize <code>GroupAlgebra</code> for now.
</p>
<blockquote class="citation">
<blockquote class="citation">
<p>
The part that goes the other way is moving the addition of the
semisimple axiom check, but I feel the code in
<code>Sets.ParentMethods.algebra</code> is largely a hack and probably should
be distributed to the appropriate subcategories.
</p>
</blockquote>
<p>
Agreed! For this use case and others, I have been wishing for a hook
in the category infrastructure enabling a category C to inspect parent
upon their initalization and provide additional categories they should
be endowed with.
</p>
<p>
Maybe we don't actually need new infrastructure, and could just use
<span class="underline">init_extra</span> and _refine_category():
</p>
<pre class="wiki"> def Groups.Finite.Algebras.ParentMethods.__init_extra__(self)
G = ...
R = ...
if ... non modular ...:
self._refine_category(...SemiSimple())
</pre><p>
Bearing that in mind, we may not want to move the code to an
intermediate spot when it's plausibly going to move again later.
</p>
</blockquote>
<p>
<code>__init_extra__</code> is run after <code>__init__</code>, correct? So it would be safe to assume the category has been initialized and <code>_refine_category</code> is suppose to play nicely in regards to <code>UniqueRepresentation</code>. So this should work and improve things.
</p>
<blockquote class="citation">
<blockquote class="citation">
<p>
While it may not be quite what you mean
</p>
</blockquote>
<p>
Sorry for the ambiguity. Let me try to clarify:
</p>
<ul><li>Indeed we don't want a coercion F -> R.F for a general CFM R.F.
Most likely we don't want a conversion either.
</li></ul><ul><li>For a group algebra, the conversion G -> R.G indeed sounds
reasonable. I am unsure at this stage about a coercion; in such
doubt my usual approach is to not put a coercion in such a case;
later it's harder to remove a coercion than add one.
</li></ul></blockquote>
<p>
Well, there is an ambiguity for additive groups for coercions, even if we consider it as an action. However, it would be strange to have notation using the group elements and not having a coercion, likely leading to bug reports. I am strongly in favor of groups having coercion and for additive groups <code>+</code> being addition of two elements in the algebra.
</p>
<blockquote class="citation">
<blockquote>
<p>
We don't need to have a concrete class <a class="missing wiki">GroupAlgebra?</a> for this. Such a
coercion could be declared in
<code>Groups.Algebra.ParentMethods.__init__extra</code>. Presumably we could
generalize it to <code>Magmas.Unital.ParentMethods.__init_extra</code> and its
additive equivalent.
</p>
</blockquote>
</blockquote>
<p>
I think there would need to be an assumption that the group elements are the indexing set of the basis, which means we need a concrete class. While this is a very reasonable assumption, it does place a restriction. Also, we are not really getting rid of a concrete class, just splitting it between CFM and the category. I feel like this obfuscates the code for little to no gain.
</p>
<blockquote class="citation">
<ul><li>The lifting of a coercion G-> H to R[G] -> R[H] for group algebras
looks reasonable indeed. Wouldn't be enough to move the
<code>construction</code> method to <code>Groups.ParentMethods</code>, and update the
functor to call <code>G.algebra(R)</code> rather than <code>GroupAlgebra(G,R)</code>?
(I haven't tried).
</li></ul></blockquote>
<p>
I don't know. I would have to experiment. However, I agree that that does sound like a plausible approach.
</p>
<blockquote class="citation">
<blockquote>
<p>
This could actually be lifted to <code>Magmas.ParentMethods</code> and its
additive counterpart.
</p>
</blockquote>
<p>
With that, the concrete class "<a class="missing wiki">GroupAlgebra?</a>" could be replaced by a
simple alias <code>GroupAlgebra(G,R)</code> -> <code>G.algebra(R)</code>, right?
</p>
</blockquote>
<blockquote class="citation">
<blockquote class="citation">
<p>
If we decide to keep this output but go to a generic CFM with
category implementation, then we can pass the appropriate parameters
during the construction. So the output is an independent question.
</p>
</blockquote>
<p>
+1. This could be done in Magmas.Algebras.<span class="underline">init</span>extra_ with set_options.
Or we could overwrite the _repr methods in <a class="missing wiki">MagmaAlgebra?</a>.<a class="missing wiki">ElementMethods?</a>.
</p>
</blockquote>
<p>
That is just getting more complicated and starts having a code smell, with the additional assumption that group algebra elements should be written as sums over the group elements. While it is not implemented yet, we could have a subalgebra of a matrix algebra being a group algebra for a matrix group.
</p>
<blockquote class="citation">
<blockquote class="citation">
<p>
I'm in favor of the output of <code>GroupAlgebra</code>
</p>
</blockquote>
<p>
I agree that the repr of <a class="missing wiki">GroupAlgebra?</a> is nicely concise and close to
usual math notations; so that could be a good option.
</p>
<p>
Opinions anyone? Are there are cases where there could be ambiguity
and we would want something slightly more verbose?
</p>
<blockquote class="citation">
<p>
and in an ideal world, it would check the repr atomicness of the
basis keys.
</p>
</blockquote>
<p>
Not sure what you mean, but that's fine :-)
</p>
</blockquote>
<p>
For example, the algebra for the weight lattice, the element <code>La[1] + La[2]</code> is ambiguous. Is it <code>1 * (La[1]) + 1*(La[2])</code> or <code>1*(La[1]+La[2])</code>. I hack around this currently by going back to the old notation when the group is in <code>ModulesWithBasis</code>.
</p>
<p>
I feel that ideally this would be done by using <code>_repr_options('element_is_atomic')</code>, but that might be quite the correct hook.
</p>
TicketnthieryFri, 05 May 2017 20:48:54 GMT
https://trac.sagemath.org/ticket/18700#comment:18
https://trac.sagemath.org/ticket/18700#comment:18
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/18700#comment:17" title="Comment 17">tscrim</a>:
</p>
<blockquote class="citation">
<p>
Well, I am not sure we are going to be able to escape the coercion
issue. At least for now, because <code>GroupAlgebra</code> will be a small
extra layer over CFM and there could be other thorny issues with
getting the functionality we'd want, I think we should utilize
<code>GroupAlgebra</code> for now.
</p>
</blockquote>
<p>
Ok, let's see.
</p>
<blockquote class="citation">
<p>
<code>__init_extra__</code> is run after <code>__init__</code>, correct?
So it would be
safe to assume the category has been initialized
</p>
</blockquote>
<p>
It's run just at the end of <code>Parent.__init__()</code>. So indeed, the
category should be initalized then.
</p>
<blockquote class="citation">
<p>
Well, there is an ambiguity for additive groups for coercions, even
if we consider it as an action. However, it would be strange to have
notation using the group elements and not having a coercion, likely
leading to bug reports. I am strongly in favor of groups having
coercion and for additive groups <code>+</code> being addition of two elements
in the algebra.
</p>
</blockquote>
<p>
Good point. So we absolutely don't want coercion in the additive case.
conversion is probably fine. Remains to decide whether we want just
conversion or also coercoin for
</p>
<p>
In any cases, we can just choose between the various options in the
<code>__init_extra__</code> of <code>Magmas.ParentMethods</code> and
<code>AdditiveMagmas.ParentMethods</code>. So no issue here beside deciding what
we want.
</p>
<blockquote class="citation">
<blockquote class="citation">
<blockquote>
<p>
We don't need to have a concrete class <a class="missing wiki">GroupAlgebra?</a> for this. Such a
coercion could be declared in
<code>Groups.Algebra.ParentMethods.__init__extra</code>. Presumably we could
generalize it to <code>Magmas.Unital.ParentMethods.__init_extra</code> and its
additive equivalent.
</p>
</blockquote>
</blockquote>
</blockquote>
<blockquote class="citation">
<p>
I think there would need to be an assumption that the group elements
are the indexing set of the basis, which means we need a concrete
class. While this is a very reasonable assumption, it does place a
restriction. Also, we are not really getting rid of a concrete
class, just splitting it between CFM and the category. I feel like
this obfuscates the code for little to no gain.
</p>
</blockquote>
<p>
In fact the code of Groups.Algebras already makes in a couple places
the assumption that the canonical basis is indexed by the group. This
is specified semi-explicitly in the documentation:
</p>
<pre class="wiki"> XXX.Algebras:
Return the category of objects constructed as algebras of
objects of ``self`` over ``base_ring``.
</pre><p>
The important part is "constructed as", the more specific definition
of it being given in xxx.algebra().
</p>
<p>
We can discuss the merit of having this assumption. I believe we
definitely need a category for "group algebras over their canonical
basis". Arguably it could possibly be useful to also have a more
general category for just "group algebras"; and then it's debatable
whether Groups.Algebras should be the former or the latter. I would
tend to just keep the current assumption for now.
</p>
<blockquote class="citation">
<p>
That is just getting more complicated and starts having a code
smell, with the additional assumption that group algebra elements
should be written as sums over the group elements. While it is not
implemented yet, we could have a subalgebra of a matrix algebra
being a group algebra for a matrix group.
</p>
</blockquote>
<p>
Does your perspective change with the above precision about what
the category <code>Group.Algebras</code> is about?
</p>
<blockquote class="citation">
<p>
For example, the algebra for the weight lattice, the element `La[1]
+ La[2]<code> is ambiguous. Is it </code>1 * (La[1]) + 1*(La[2])` or
<code>1*(La[1]+La[2])</code>. I hack around this currently by going back to the
old notation when the group is in <code>ModulesWithBasis</code>.
</p>
</blockquote>
<p>
I see. We could also get away with it with some "in doubt add
parenthesis" heuristic; E.g. if the string repr of term contains a
<code>+</code>. Not great, but simple and safe (for output purposes).
</p>
<blockquote class="citation">
<p>
I feel that ideally this would be done by using
</p>
<blockquote>
<p>
<code>_repr_options('element_is_atomic')</code>, but that might be quite the
correct hook.
</p>
</blockquote>
</blockquote>
<p>
Ok.
</p>
TickettscrimSat, 06 May 2017 19:58:39 GMT
https://trac.sagemath.org/ticket/18700#comment:19
https://trac.sagemath.org/ticket/18700#comment:19
<p>
Okay, there is a technical limitation to using the <code>__init_extra__</code>: percolating calls up through the category methods.
</p>
<p>
I don't quite agree with "constructed as" meaning the basis must be indexed by the group. I also don't think we should enforce that, and only place I currently see that assumption used is the default implementation of <code>group</code>. I also do not want a category for a single object in a more natural category: it should be a concrete class. Although right now, this is moot (see below).
</p>
<p>
I've moved the methods that I could up to the category. <code>construction</code> cannot be lifted because the default in <code>Parent</code> is to <code>return None</code>. Similar for <code>is_exact</code>. So I believe we need a concrete class
</p>
<p>
So I think at this point, this is the best we are really going to get. We can address some of these issues on some follow-up tickets.
</p>
TicketgitSat, 06 May 2017 19:58:48 GMTcommit changed
https://trac.sagemath.org/ticket/18700#comment:20
https://trac.sagemath.org/ticket/18700#comment:20
<ul>
<li><strong>commit</strong>
changed from <em>2ef2208407ec7edfbe03a8c9980741f25fc2db2a</em> to <em>647264f6107ad29f987821e4a585c36215e49e40</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1. New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="https://git.sagemath.org/sage.git/commit/?id=c173ff81994d64baf2547aef431e0f193de41860"><span class="icon"></span>c173ff8</a></td><td><code>Merge branch 'public/groups/standardize_group_algebras-18700' of git://trac.sagemath.org/sage into public/groups/standardize_group_algebras-18700</code>
</td></tr><tr><td><a class="ext-link" href="https://git.sagemath.org/sage.git/commit/?id=647264f6107ad29f987821e4a585c36215e49e40"><span class="icon"></span>647264f</a></td><td><code>Lifing some methods up to the category.</code>
</td></tr></table>
TickettscrimSat, 06 May 2017 20:18:06 GMTstatus changed
https://trac.sagemath.org/ticket/18700#comment:21
https://trac.sagemath.org/ticket/18700#comment:21
<ul>
<li><strong>status</strong>
changed from <em>needs_info</em> to <em>needs_review</em>
</li>
</ul>
TicketnthierySun, 07 May 2017 21:04:53 GMT
https://trac.sagemath.org/ticket/18700#comment:22
https://trac.sagemath.org/ticket/18700#comment:22
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/18700#comment:19" title="Comment 19">tscrim</a>:
</p>
<blockquote class="citation">
<p>
I don't quite agree with "constructed as" meaning the basis must be indexed by the group. I also don't think we should enforce that, and only place I currently see that assumption used is the default implementation of <code>group</code>. I also do not want a category for a single object in a more natural category: it should be a concrete class. Although right now, this is moot (see below).
</p>
</blockquote>
<p>
That's the same with other functorial constructions like cartesian
product. It gives you the cartesian product in its canonical
representation.
</p>
<p>
Here are other methods (or to be implemented methods) that use the
assumption that the group algebra is represented in its canonical
basis:
</p>
<ul><li>default implementation of algebra_generators
</li><li>center_basis
</li><li>multiplication
</li><li>coproduct, antipode, counit,
</li><li>stuff about representation theory of monoids
</li><li>construction of the Jucys Murphy elements for the symmetric group
</li></ul><p>
Note that those live at different levels (magmas, monoids, groups,
specific groups). So if really want to support both "constructed as"
and "isomorphic to", a functorial construction and a concrete class
won't be enough. We need two functorial constructions.
</p>
<blockquote class="citation">
<p>
I've moved the methods that I could up to the category.
</p>
</blockquote>
<p>
Cool, thanks!
</p>
<blockquote class="citation">
<p>
<code>construction</code> cannot be lifted because the default in <code>Parent</code> is to <code>return None</code>.
</p>
</blockquote>
<p>
This is just the usual artifact that for historical reasons, there is
too much stuff in Parent, which prevents categories to provide default
implementations. <code>construction</code> being a non speed-critical method, I
foresee no obstruction to moving it to Sets (but need to double check
the code first).
</p>
<blockquote class="citation">
<p>
Similar for <code>is_exact</code>.
</p>
</blockquote>
<p>
This one is a bit more annoying since it's a cpdef method. Worst comes
to worst, we could do as for <span class="underline">getitem</span>.
</p>
<blockquote class="citation">
<p>
So I believe we need a concrete class
</p>
</blockquote>
<p>
Not yet clear :-)
</p>
TicketnthierySun, 07 May 2017 22:14:30 GMT
https://trac.sagemath.org/ticket/18700#comment:23
https://trac.sagemath.org/ticket/18700#comment:23
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/18700#comment:19" title="Comment 19">tscrim</a>:
</p>
<blockquote class="citation">
<p>
Okay, there is a technical limitation to using the <code>__init_extra__</code>: percolating calls up through the category methods.
</p>
</blockquote>
<p>
Ok. Could you elaborate?
</p>
TicketnthieryMon, 08 May 2017 01:53:35 GMT
https://trac.sagemath.org/ticket/18700#comment:24
https://trac.sagemath.org/ticket/18700#comment:24
<p>
Fore reference, <a class="new ticket" href="https://trac.sagemath.org/ticket/11318" title="enhancement: Make GroupAlgebra(G, R) an alias for G.algebra(R), and remove its code (new)">#11318</a> is the ticket suggesting to get rid of <a class="missing wiki">GroupAlgebra?</a>.
</p>
TickettscrimMon, 08 May 2017 05:46:51 GMT
https://trac.sagemath.org/ticket/18700#comment:25
https://trac.sagemath.org/ticket/18700#comment:25
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/18700#comment:23" title="Comment 23">nthiery</a>:
</p>
<blockquote class="citation">
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/18700#comment:19" title="Comment 19">tscrim</a>:
</p>
<blockquote class="citation">
<p>
Okay, there is a technical limitation to using the <code>__init_extra__</code>: percolating calls up through the category methods.
</p>
</blockquote>
<p>
Ok. Could you elaborate?
</p>
</blockquote>
<p>
I could not use <code>super</code> to make the calls without getting either an infinite recursion or errors from Python. I also think we should not force a direct call to the unital algebra's <code>__init_extra__</code> in case one of the other base categories ends up having an <code>__init_extra__</code>.
</p>
TickettscrimMon, 08 May 2017 06:05:12 GMT
https://trac.sagemath.org/ticket/18700#comment:26
https://trac.sagemath.org/ticket/18700#comment:26
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/18700#comment:22" title="Comment 22">nthiery</a>:
</p>
<blockquote class="citation">
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/18700#comment:19" title="Comment 19">tscrim</a>:
</p>
<blockquote class="citation">
<p>
I don't quite agree with "constructed as" meaning the basis must be indexed by the group. I also don't think we should enforce that, and only place I currently see that assumption used is the default implementation of <code>group</code>. I also do not want a category for a single object in a more natural category: it should be a concrete class. Although right now, this is moot (see below).
</p>
</blockquote>
</blockquote>
<blockquote class="citation">
<p>
That's the same with other functorial constructions like cartesian
product. It gives you the cartesian product in its canonical
representation.
</p>
</blockquote>
<p>
In that case, it creates an instance of a specific concrete class.
</p>
<blockquote class="citation">
<p>
Here are other methods (or to be implemented methods) that use the
assumption that the group algebra is represented in its canonical
basis:
</p>
<ul><li>default implementation of algebra_generators
</li><li>center_basis
</li><li>multiplication
</li><li>coproduct, antipode, counit,
</li><li>stuff about representation theory of monoids
</li><li>construction of the Jucys Murphy elements for the symmetric group
</li></ul></blockquote>
<p>
I believe all of those either depend on <code>group</code>, just need a basis, or need a concrete implementation.
</p>
<blockquote class="citation">
<p>
Note that those live at different levels (magmas, monoids, groups,
specific groups). So if really want to support both "constructed as"
and "isomorphic to", a functorial construction and a concrete class
won't be enough. We need two functorial constructions.
</p>
</blockquote>
<p>
Hmm...I think that for <code>construction</code> and for our concrete implementation of that functor, that we need to make assumptions about the image. However, I think the category should be there for the isomorphic to and the constructed as should be a concrete class. Having a concrete class also makes things easier to maintain and add to for the non-initiated.
</p>
<blockquote class="citation">
<blockquote class="citation">
<p>
<code>construction</code> cannot be lifted because the default in <code>Parent</code> is to <code>return None</code>.
</p>
</blockquote>
<p>
This is just the usual artifact that for historical reasons, there is
too much stuff in Parent, which prevents categories to provide default
implementations. <code>construction</code> being a non speed-critical method, I
foresee no obstruction to moving it to Sets (but need to double check
the code first).
</p>
</blockquote>
<p>
I agree. However, it is something that is currently a limitation.
</p>
<blockquote class="citation">
<blockquote class="citation">
<p>
Similar for <code>is_exact</code>.
</p>
</blockquote>
<p>
This one is a bit more annoying since it's a cpdef method. Worst comes
to worst, we could do as for <span class="underline">getitem</span>.
</p>
</blockquote>
<p>
This might be something we could lift from <code>GroupAlgebra</code> to CFM.
</p>
<blockquote class="citation">
<blockquote class="citation">
<p>
So I believe we need a concrete class
</p>
</blockquote>
<p>
Not yet clear :-)
</p>
</blockquote>
<p>
I suspect if you had your way, nearly everything would be implemented as a category. :P
</p>
TicketnthieryMon, 08 May 2017 12:42:34 GMT
https://trac.sagemath.org/ticket/18700#comment:27
https://trac.sagemath.org/ticket/18700#comment:27
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/18700#comment:25" title="Comment 25">tscrim</a>:
</p>
<blockquote class="citation">
<p>
I could not use <code>super</code> to make the calls without getting either an infinite recursion or errors from Python. I also think we should not force a direct call to the unital algebra's <code>__init_extra__</code> in case one of the other base categories ends up having an <code>__init_extra__</code>.
</p>
</blockquote>
<p>
In principle there is no need for super. By design, <code>C.ParentMethods__init_extra__</code> is providing *additional* initialization code that should be run for every parent in C or some subcategory. Hence the specific protocol: the <code>__init_extra__</code> of each and every category is being called, without having to do an explicit super call.
</p>
TicketgitMon, 08 May 2017 13:29:27 GMTcommit changed
https://trac.sagemath.org/ticket/18700#comment:28
https://trac.sagemath.org/ticket/18700#comment:28
<ul>
<li><strong>commit</strong>
changed from <em>647264f6107ad29f987821e4a585c36215e49e40</em> to <em>23772ed94b1f063091a3ab54f55dc8d56abba5e7</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1. New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="https://git.sagemath.org/sage.git/commit/?id=23772ed94b1f063091a3ab54f55dc8d56abba5e7"><span class="icon"></span>23772ed</a></td><td><code>Better handling additive groups.</code>
</td></tr></table>
TickettscrimMon, 08 May 2017 13:34:51 GMT
https://trac.sagemath.org/ticket/18700#comment:29
https://trac.sagemath.org/ticket/18700#comment:29
<p>
I first tried that, and I must have mistested something. I was able to use <code>__init_extra__</code> to refine the category and looked like a good hook to do similar things in the future.
</p>
<p>
However, I decided not to do that because we would have to duplicate it between <code>Groups</code> and <code>AdditiveGroups</code>. Perhaps that will be one of the best arguments to you for not moving everything to the category. Now I'm considering pulling back the methods I lifted to the category for this reason. Thoughts?
</p>
TicketnthieryMon, 08 May 2017 19:18:51 GMT
https://trac.sagemath.org/ticket/18700#comment:30
https://trac.sagemath.org/ticket/18700#comment:30
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/18700#comment:29" title="Comment 29">tscrim</a>:
</p>
<blockquote class="citation">
<p>
I first tried that, and I must have mistested something. I was able to use <code>__init_extra__</code> to refine the category and looked like a good hook to do similar things in the future.
</p>
<p>
However, I decided not to do that because we would have to duplicate it between <code>Groups</code> and <code>AdditiveGroups</code>. Perhaps that will be one of the best arguments to you for not moving everything to the category. Now I'm considering pulling back the methods I lifted to the category for this reason. Thoughts?
</p>
</blockquote>
<p>
The duplication between <code>Groups</code> and <code>AdditiveGroups</code> here and for all the rest is annoying, but at this stage this is the price we pay. I am actually investigating with colleagues ways to avoid this; see:
</p>
<ul><li><a class="ext-link" href="http://opendreamkit.org/2016/08/01/CICM/"><span class="icon"></span>http://opendreamkit.org/2016/08/01/CICM/</a>
</li><li><a class="ext-link" href="http://opendreamkit.org/2016/08/01/CICM/#the-additive-structures-hierarchy-from-its-multiplicative-counterpart"><span class="icon"></span>http://opendreamkit.org/2016/08/01/CICM/#the-additive-structures-hierarchy-from-its-multiplicative-counterpart</a>
</li></ul><p>
but let's not hold our breaths on this.
</p>
<p>
In the meantime, we can reduce the duplication to a minima by using an alias:
</p>
<pre class="wiki"> class AdditiveGroups:
class Algebras:
class ParentMethods:
__init_extra__ = Groups.Algebras.ParentMethods.__init_extra__.im_func
</pre><p>
Also: we may want to treat additive and multiplicative slightly differently, since coercion is plausible in the latter but not the former case. So maybe it won't be exact duplication.
</p>
TicketnthieryMon, 08 May 2017 20:05:24 GMT
https://trac.sagemath.org/ticket/18700#comment:31
https://trac.sagemath.org/ticket/18700#comment:31
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/18700#comment:26" title="Comment 26">tscrim</a>:
</p>
<blockquote class="citation">
<p>
In that case, it creates an instance of a specific concrete class.
</p>
</blockquote>
<p>
Yes because the cartesian product construction requires a specific
data structure. Note that there is no concrete class
<code>CartesianProduct</code> for e.g. groups. Just one for sets (and one for
modules since we use a different data structure then).
</p>
<p>
In our case we don't have to define a new datastructure since we can
just use that of e.g. CFM.
</p>
<blockquote class="citation">
<p>
I believe all of those either depend on <code>group</code>, just need a basis, or need a concrete implementation.
</p>
</blockquote>
<p>
Their current implementation all require the algebra to be expressed
in its canonical basis. Yes that's pretty specific. Yet not quite
concrete. We currently are using <a class="missing wiki">CombinatorialFreeModule?</a> for the
concrete data structure, but could switch to any other
<a class="missing wiki">ModulesWithBasis?</a> implementation.
</p>
<blockquote class="citation">
<p>
I think the category should be there for the "isomorphic to" and the
"constructed as" should be a concrete class. Having a concrete class
also makes things easier to maintain and add to for the non-initiated.
</p>
</blockquote>
<p>
But then almost all the methods I mentioned above need to go down in
the concrete class. And then we actually need several concrete
classes: <a class="missing wiki">MagmaAlgebra?</a>, <a class="missing wiki">SemigroupAlgebra?</a>, <a class="missing wiki">MonoidAlgebra?</a>, <a class="missing wiki">GroupAlgebra?</a>,
JTrivialMonoidAlgebra, all with appropriate inheritance. Thus just
redoing by hand what categories do for us, and not really making life
simpler for the non-initiated.
</p>
<blockquote class="citation">
<blockquote class="citation">
<blockquote class="citation">
<p>
<code>construction</code> cannot be lifted because the default in <code>Parent</code> is to <code>return None</code>.
</p>
</blockquote>
</blockquote>
<p>
I agree. However, it is something that is currently a limitation.
</p>
</blockquote>
<p>
Right, we have to do something. Let's just move the default
<code>construction</code> from <code>Parent</code> to <code>Sets</code>. I am happy giving it a shot.
</p>
<blockquote class="citation">
<p>
... about <code>is_exact</code>.
This might be something we could lift from <code>GroupAlgebra</code> to CFM.
</p>
</blockquote>
<p>
Very good point. It really would belong go Modules.<a class="missing wiki">WithBasis?</a>, but CFM
is a pretty good approximation for now.
</p>
<blockquote class="citation">
<p>
I suspect if you had your way, nearly everything would be implemented as a category. :P
</p>
</blockquote>
<p>
:-)
</p>
<p>
You know, the category infrastructure grew out of struggling with
concrete use cases (e.g. when implementing cartesian products), seeing
the same specific pattern come again and again, and redoing by hand
the same stuff that did not scale. And having in mind what other had
done in other systems to tackle the same pattern. I invested two years
worth of coding into that, not to have to struggle again. So yes, when
I see the specific pattern, I am motivated to use the infrastructure!
</p>
<p>
Cheers,
</p>
TickettscrimMon, 08 May 2017 21:56:07 GMT
https://trac.sagemath.org/ticket/18700#comment:32
https://trac.sagemath.org/ticket/18700#comment:32
<p>
It will be very good if you're able to remove the duplication between <code>+</code> and <code>*</code> groups. :)
</p>
<p>
*sigh* too much is already shoved up with assumptions into the categories. I <em>really</em> do not like this fragmented code; I have a hard time figuring out what is doing what where. In addition, I see the requirement for a specific basis as being a concrete class, and we have ill-specified things between the abstract class and the concrete class. I also believe that by having <code>GroupAlgebra</code> for now makes dealing with the code duplication between <code>+</code>/<code>*</code> groups much better (we don't have to worry about making an alias for everything). At this point, we do not have a need for a hierarchy of <code>*Algebra</code>, but that could eventually be a problem.
</p>
<p>
At this point, I will just lift everything up to the category under strong objections, but I would rather see this ticket get merged then left in limbo as it fixes a number of things with group algebras.
</p>
TicketnthieryTue, 09 May 2017 00:40:28 GMT
https://trac.sagemath.org/ticket/18700#comment:33
https://trac.sagemath.org/ticket/18700#comment:33
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/18700#comment:32" title="Comment 32">tscrim</a>:
</p>
<blockquote class="citation">
<p>
I <em>really</em> do not like this fragmented code;
</p>
</blockquote>
<p>
I see your point. But that's not the code, it's just the math behind. When writing a math book, each theorem comes with its own combination of hypotheses. The author can simplify the reader's life by taking some common strong assumption for all theorems in the book (e.g. all fields are algebraicall closed and of char 0). But that's at the price of generality.
</p>
<p>
That's what's happening to us. For a long time, systems afforded to make strong assumptions (like all rings are commutative). But Sage aims beyond and enable the study of a much wider variety of objects.
</p>
<blockquote class="citation">
<p>
I have a hard time figuring out what is doing what where.
</p>
</blockquote>
<p>
Given a specific method, it's relatively straightforward with the current infrastructure to decide where to put it (figuring the hypotheses of the theorem). I agree that getting the big picture is hard. We need better tools to explore and visualize what's implemented.
</p>
<blockquote class="citation">
<p>
In addition, I see the requirement for a specific basis as being a concrete class, and we have ill-specified things between the abstract class and the concrete class.
</p>
</blockquote>
<p>
Somehow, putting everything about xxx algebras in the category simplifies things. No need to worry anymore where things should be :-)
</p>
<blockquote class="citation">
<p>
I also believe that by having <code>GroupAlgebra</code> for now makes dealing with the code duplication between <code>+</code>/<code>*</code> groups much better (we don't have to worry about making an alias for everything).
</p>
</blockquote>
<p>
Let's see how it goes. Before the ticket, <code>GroupAlgebra</code> only really supported multiplicative groups, right? If we only lift its features to multiplicative structures for now, that's ok.
</p>
<blockquote class="citation">
<p>
At this point, we do not have a need for a hierarchy of <code>*Algebra</code>, but that could eventually be a problem.
</p>
</blockquote>
<p>
If the aim is only to support group algebras, we indeed don't need that hierarchy. Strangely enough, I very much care about supporting semigroups and monoids of all sorts :-)
</p>
<blockquote class="citation">
<p>
At this point, I will just lift everything up to the category under strong objections, but I would rather see this ticket get merged then left in limbo as it fixes a number of things with group algebras.
</p>
</blockquote>
<p>
Ok, thanks. Sorry for twisting your arm. I need to improve my arguments to be more convincing :-)
</p>
<p>
Let me know if there is anything I can do to help.
</p>
<p>
Cheers,
</p>
TicketgitTue, 09 May 2017 03:29:18 GMTcommit changed
https://trac.sagemath.org/ticket/18700#comment:34
https://trac.sagemath.org/ticket/18700#comment:34
<ul>
<li><strong>commit</strong>
changed from <em>23772ed94b1f063091a3ab54f55dc8d56abba5e7</em> to <em>9e77b87f234377b3d72f94abd293af15cfc7a22c</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1. New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="https://git.sagemath.org/sage.git/commit/?id=ee8b0c15336c25c646bb176f0c8c493ecaa7b586"><span class="icon"></span>ee8b0c1</a></td><td><code>Moving more stuff from GroupAlgebra to the categories.</code>
</td></tr><tr><td><a class="ext-link" href="https://git.sagemath.org/sage.git/commit/?id=427179a6a316f771e1e6286b7e1b5f705d66bb9b"><span class="icon"></span>427179a</a></td><td><code>Moving generic construction to the category.</code>
</td></tr><tr><td><a class="ext-link" href="https://git.sagemath.org/sage.git/commit/?id=9e77b87f234377b3d72f94abd293af15cfc7a22c"><span class="icon"></span>9e77b87</a></td><td><code>Trying to move things up to the category as much as possible. Still WIP.</code>
</td></tr></table>
TickettscrimTue, 09 May 2017 03:44:51 GMTstatus changed
https://trac.sagemath.org/ticket/18700#comment:35
https://trac.sagemath.org/ticket/18700#comment:35
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>needs_work</em>
</li>
</ul>
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/18700#comment:33" title="Comment 33">nthiery</a>:
</p>
<blockquote class="citation">
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/18700#comment:32" title="Comment 32">tscrim</a>:
</p>
<blockquote class="citation">
<p>
I <em>really</em> do not like this fragmented code;
</p>
</blockquote>
<p>
I see your point. But that's not the code, it's just the math behind. When writing a math book, each theorem comes with its own combination of hypotheses. The author can simplify the reader's life by taking some common strong assumption for all theorems in the book (e.g. all fields are algebraicall closed and of char 0). But that's at the price of generality.
</p>
<p>
That's what's happening to us. For a long time, systems afforded to make strong assumptions (like all rings are commutative). But Sage aims beyond and enable the study of a much wider variety of objects.
</p>
</blockquote>
<p>
However, in math books/papers, we do not require the same amount of specificity that code needs for computations nor the distinction between isomorphism and equality.
</p>
<blockquote class="citation">
<blockquote class="citation">
<p>
In addition, I see the requirement for a specific basis as being a concrete class, and we have ill-specified things between the abstract class and the concrete class.
</p>
</blockquote>
<p>
Somehow, putting everything about xxx algebras in the category simplifies things. No need to worry anymore where things should be :-)
</p>
<blockquote class="citation">
<p>
I also believe that by having <code>GroupAlgebra</code> for now makes dealing with the code duplication between <code>+</code>/<code>*</code> groups much better (we don't have to worry about making an alias for everything).
</p>
</blockquote>
<p>
Let's see how it goes. Before the ticket, <code>GroupAlgebra</code> only really supported multiplicative groups, right? If we only lift its features to multiplicative structures for now, that's ok.
</p>
</blockquote>
<p>
Yes, but everything still worked with additive groups once <code>algebras</code> redirected to construct them in that case.
</p>
<blockquote class="citation">
<blockquote class="citation">
<p>
At this point, we do not have a need for a hierarchy of <code>*Algebra</code>, but that could eventually be a problem.
</p>
</blockquote>
<p>
If the aim is only to support group algebras, we indeed don't need that hierarchy. Strangely enough, I very much care about supporting semigroups and monoids of all sorts :-)
</p>
</blockquote>
<p>
I never, ever would have guessed. :P
</p>
<blockquote class="citation">
<blockquote class="citation">
<p>
At this point, I will just lift everything up to the category under strong objections, but I would rather see this ticket get merged then left in limbo as it fixes a number of things with group algebras.
</p>
</blockquote>
<p>
Ok, thanks. Sorry for twisting your arm. I need to improve my arguments to be more convincing :-)
</p>
</blockquote>
<p>
It's not that your arguments are not valid with lots of experience, but I see things from a slightly different perspective. I appreciate your point of view and find these debates productive and beneficial (if slightly frustrating at times :P).
</p>
<blockquote class="citation">
<p>
Let me know if there is anything I can do to help.
</p>
</blockquote>
<p>
I've done what I could for right now. I've pushed my current WIP. If you could finish up getting the coercion working, it didn't work for me as-is (with renaming <code>GroupAlgebra._coerce_map_from_</code>). I think the <code>_element_constructor_</code> is close enough to the CFM that we can remove it, but I haven't tried it yet. The only other thing is moving some documentation and tests around to get this done. Thanks.
</p>
TicketnthieryThu, 11 May 2017 03:16:49 GMT
https://trac.sagemath.org/ticket/18700#comment:36
https://trac.sagemath.org/ticket/18700#comment:36
<p>
Will have a look now.
</p>
TicketnthieryThu, 11 May 2017 20:14:28 GMT
https://trac.sagemath.org/ticket/18700#comment:37
https://trac.sagemath.org/ticket/18700#comment:37
<p>
Still working on it :-)
</p>
TickettscrimThu, 11 May 2017 20:30:19 GMT
https://trac.sagemath.org/ticket/18700#comment:38
https://trac.sagemath.org/ticket/18700#comment:38
<p>
Thanks. :)
</p>
TicketgitFri, 12 May 2017 04:39:21 GMTcommit changed
https://trac.sagemath.org/ticket/18700#comment:39
https://trac.sagemath.org/ticket/18700#comment:39
<ul>
<li><strong>commit</strong>
changed from <em>9e77b87f234377b3d72f94abd293af15cfc7a22c</em> to <em>d55147b6a79289df93453fdc763a8a143e86cdac</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1. New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="https://git.sagemath.org/sage.git/commit/?id=d55147b6a79289df93453fdc763a8a143e86cdac"><span class="icon"></span>d55147b</a></td><td><code>backup</code>
</td></tr></table>
TicketnthieryFri, 12 May 2017 04:42:07 GMT
https://trac.sagemath.org/ticket/18700#comment:40
https://trac.sagemath.org/ticket/18700#comment:40
<p>
For info: I pushed my current branch if you want to have a look and for backup. But please don't build on it: I *will* rewrite history (I should have done it on a private branch but screwed up. It's over time to go to bed!).
</p>
TicketgitFri, 12 May 2017 21:33:01 GMTcommit changed
https://trac.sagemath.org/ticket/18700#comment:41
https://trac.sagemath.org/ticket/18700#comment:41
<ul>
<li><strong>commit</strong>
changed from <em>d55147b6a79289df93453fdc763a8a143e86cdac</em> to <em>045367eeb1323944891aa78de1eecb3ec9b443cd</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1. New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="https://git.sagemath.org/sage.git/commit/?id=045367eeb1323944891aa78de1eecb3ec9b443cd"><span class="icon"></span>045367e</a></td><td><code>backup2</code>
</td></tr></table>
TicketnthieryFri, 12 May 2017 21:39:08 GMT
https://trac.sagemath.org/ticket/18700#comment:42
https://trac.sagemath.org/ticket/18700#comment:42
<p>
All right: the scary failures I was getting are now fixed (or worked
around for one of them). More details later on. Unless I accidently
broke new files (I only checked with --failed), most tests should now
pass.
</p>
<p>
So long <code>GroupAlgebra</code>, and thanks for the fish!
</p>
<p>
Again, don't build on the current commits: I just stuffed everything
in there for backup and early release purposes. I am now planning to
rewrite history to split the stuff in intelligible chunks to ease the
review. Hopefully tonight, but no promise; nice weather here, time for
a family walk on Mont Royal!
</p>
<p>
Cheers,
</p>
TicketnthieryFri, 12 May 2017 23:48:22 GMT
https://trac.sagemath.org/ticket/18700#comment:43
https://trac.sagemath.org/ticket/18700#comment:43
<p>
For the record, here is the outcome of <code>make ptestlong</code>:
</p>
<pre class="wiki">----------------------------------------------------------------------
sage -t --long src/sage/categories/semigroups.py # 3 doctests failed
sage -t --long src/sage/algebras/quatalg/quaternion_algebra.py # 1 doctest failed
sage -t --long src/sage/calculus/calculus.py # 1 doctest failed
sage -t --long src/sage/groups/perm_gps/permgroup_named.py # 1 doctest failed
sage -t --long src/sage/combinat/permutation.py # 1 doctest failed
sage -t --long src/sage/combinat/symmetric_group_algebra.py # 3 doctests failed
sage -t --long src/sage/categories/modules_with_basis.py # 1 doctest failed
sage -t --long src/sage/categories/modules.py # 1 doctest failed
sage -t --long src/sage/categories/monoids.py # 2 doctests failed
sage -t --long src/sage/combinat/species/series.py # 1 doctest failed
sage -t --long src/sage/categories/category_with_axiom.py # 1 doctest failed
</pre><p>
I glanced through them, and most seem trivial output updates; the one in calculus is unrelated (fails for me also on develop).
</p>
TicketchapotonSat, 13 May 2017 06:22:06 GMT
https://trac.sagemath.org/ticket/18700#comment:44
https://trac.sagemath.org/ticket/18700#comment:44
<p>
The calculus one is related to a french locale LANG and giac changing its keywords to French in that context. It can be made silent by changing the locale. See <a class="closed ticket" href="https://trac.sagemath.org/ticket/22833" title="defect: fix a calculus doctest (giac, laplace, integration) (closed: fixed)">#22833</a>.
</p>
TicketnthierySat, 13 May 2017 12:41:35 GMT
https://trac.sagemath.org/ticket/18700#comment:45
https://trac.sagemath.org/ticket/18700#comment:45
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/18700#comment:44" title="Comment 44">chapoton</a>:
</p>
<blockquote class="citation">
<p>
The calculus one is related to a french locale LANG and giac changing its keywords to French in that context. It can be made silent by changing the locale. See <a class="closed ticket" href="https://trac.sagemath.org/ticket/22833" title="defect: fix a calculus doctest (giac, laplace, integration) (closed: fixed)">#22833</a>.
</p>
</blockquote>
<p>
Good to know. Thanks for the info!
</p>
TicketnthierySat, 13 May 2017 13:26:21 GMT
https://trac.sagemath.org/ticket/18700#comment:46
https://trac.sagemath.org/ticket/18700#comment:46
<p>
Before I proceed to finalization, I have a few questions:
</p>
<ul><li>We currently have tutorial material about group algebras (and more
generally XXX algebras) in several locations: Groups.Algebras,
Sets.<a class="missing wiki">ParentMethods?</a>.algebra, <a class="missing wiki">GroupAlgebras?</a>. Possibly even in the
group algebra functor. Each tutorial is more or less complete, and
there is quite some duplication. Instead, it would be best to have a
single tutorial that would take the best of each of the current
ones, and have all other locations refer to it. And we could by the
way refer to from the index of thematic tutorials.
</li></ul><blockquote>
<p>
In which of the locations do we want to put that tutorial? I would
tend to choose <code>Sets.ParentMethods.algebra</code>, as it feel the easiest
to discover as <code>G.algebra?</code> when the user has a group <code>G</code> under hand.
</p>
</blockquote>
<blockquote>
<p>
What do you think?
</p>
</blockquote>
<blockquote>
<p>
<code>GroupAlgebra</code> would also be a fairly natural entry point; but I
would tend to encourage the user of <code>G.algebra()</code> and eventually
deprecate <code>GroupAlgebra</code> at some point to remove one more thing from
the global name space. This later point is subject to debate though.
</p>
</blockquote>
<ul><li>Due to the change of perspective in the middle of the development of
this ticket (my fault), there is a bit of back and forth in the
history, which makes it hard to get an overview of what's going on.
Also there are quite a few changes that end up being undone later on
which increases the risks of conflicts with other branches.
</li></ul><blockquote>
<p>
Since this branch is based on a single develop, I am tempted to not
only rewrite the history of my latest changes, but rewrite the
complete history (squashing it all, and then redoing commits per
topic: preliminary changes like for "Parent.construction", moving
<a class="missing wiki">GroupAlgebra?</a>'s content to the categories, trivial documentation
updates, ...).
</p>
</blockquote>
<p>
. Presumably no one other than us has been working on this branch, so
</p>
<blockquote>
<p>
this should be safe even if the branch has been published. Rewriting
history is not yet usual practice in Sage (but very much is in other
projects), so I am hesitating.
</p>
</blockquote>
<blockquote>
<p>
Note: this would also mean that the commits would be under my name.
That's unfair in the git history. On the other hand, our practice is
to track credit through the trac author/reviewer fields, so that
should be alright.
</p>
</blockquote>
<blockquote>
<p>
What do you think?
</p>
</blockquote>
<ul><li>With this ticket, <code>GroupAlgebra(SymmetricGroup(3),QQ)</code> now properly
constructs an instance of <code>SymmetricGroupAlgebra(3,Q)</code>. Given that
<code>SymmetricGroupAlgebra</code> has a custom <code>_repr_</code>, this induces quite a
few trivial doctest updates, from:
<pre class="wiki"> Group algebra of Symmetric group of order 3! as a permutation group over Rational Field
</pre>to:
<pre class="wiki"> Symmetric group algebra of order 3 over Rational Field
</pre></li></ul><p>
. While we are at it, I was wondering whether we did not want to use
</p>
<blockquote>
<p>
the occasion to remove that custom repr for consistency, especially
since the that custom repr hides which specific implementation of
the symmetric group algebra it used:
</p>
<pre class="wiki"> sage: SymmetricGroupAlgebra(QQ, WeylGroup(["A",3]))
Symmetric group algebra of order 4 over Rational Field
</pre></blockquote>
<blockquote>
<p>
Maybe we would want to also change the generic repr to produce:
</p>
<pre class="wiki"> XXX group algebra over QQ
</pre><p>
instead of
</p>
<pre class="wiki"> Group algebra of XXX group over QQ
</pre></blockquote>
<blockquote>
<p>
Opinions?
</p>
</blockquote>
<ul><li>We have been struck once again in this ticket by the dreaded
category MRO issue. I have one commit in <code>sage.misc.c3_controlled</code>
which makes things more robust in waiting for a proper
implementation of <a class="new ticket" href="https://trac.sagemath.org/ticket/22962" title="enhancement: Toward singleton categories: subcategories of Modules (new)">#22962</a>.
</li></ul><blockquote>
<p>
Should I move this commit to a separate ticket?
</p>
</blockquote>
<ul><li>Lifting <code>Parent.construction</code> to <code>Sets.ParentMethods</code> forced me to
fix a couple parents to properly initialize their categories (that's
a good thing!). Should I move this, and the lifting of
"construction", to a separate ticket?
</li></ul><p>
Cheers,
</p>
<blockquote>
<p>
Nicolas
</p>
</blockquote>
TickettscrimSat, 13 May 2017 16:47:30 GMT
https://trac.sagemath.org/ticket/18700#comment:47
https://trac.sagemath.org/ticket/18700#comment:47
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/18700#comment:46" title="Comment 46">nthiery</a>:
</p>
<blockquote class="citation">
<ul><li>We currently have tutorial material about group algebras (and more
generally XXX algebras) in several locations: Groups.Algebras,
Sets.<a class="missing wiki">ParentMethods?</a>.algebra, <a class="missing wiki">GroupAlgebras?</a>. Possibly even in the
group algebra functor. Each tutorial is more or less complete, and
there is quite some duplication. Instead, it would be best to have a
single tutorial that would take the best of each of the current
ones, and have all other locations refer to it. And we could by the
way refer to from the index of thematic tutorials.
</li></ul><blockquote>
<p>
In which of the locations do we want to put that tutorial? I would
tend to choose <code>Sets.ParentMethods.algebra</code>, as it feel the easiest
to discover as <code>G.algebra?</code> when the user has a group <code>G</code> under hand.
</p>
</blockquote>
<blockquote>
<p>
What do you think?
</p>
</blockquote>
</blockquote>
<p>
Well, none of this will be available from <code>G?</code>, so I would put it in <code>GroupAlgebra</code>. However, there are two slightly different topics. The first would be basic usage and properties, the other would be provide doctests of functionality. What used to be in the module-level doc of <code>algebras/group_algebra.py</code> is the latter. For this, a natural place is where we provide the majority of the implementation: the <code>Groups.Algebras</code> category. In <code>Sets.ParentMethods.algebra</code>, we should give the necessary information to construct the object as it is a generic method.
</p>
<blockquote class="citation">
<blockquote>
<p>
<code>GroupAlgebra</code> would also be a fairly natural entry point; but I
would tend to encourage the user of <code>G.algebra()</code> and eventually
deprecate <code>GroupAlgebra</code> at some point to remove one more thing from
the global name space. This later point is subject to debate though.
</p>
</blockquote>
<ul><li>Due to the change of perspective in the middle of the development of
this ticket (my fault), there is a bit of back and forth in the
history, which makes it hard to get an overview of what's going on.
Also there are quite a few changes that end up being undone later on
which increases the risks of conflicts with other branches.
</li></ul></blockquote>
<p>
This will not cause any conflicts if you do a merge as git tries the final version, not each individual commit. It is only a potential problem if you do a rebase. IMO, it is better to have the history where you make the first attempt in case there is something we might want to salvage from it later on down the road.
</p>
<blockquote class="citation">
<blockquote>
<p>
Since this branch is based on a single develop, I am tempted to not
only rewrite the history of my latest changes, but rewrite the
complete history (squashing it all, and then redoing commits per
topic: preliminary changes like for "Parent.construction", moving
<a class="missing wiki">GroupAlgebra?</a>'s content to the categories, trivial documentation
updates, ...).
</p>
</blockquote>
</blockquote>
<p>
We might have to do some rewriting for moving the <code>Parent.construction</code>. Although we could just make those changes on a separate branch and then merge that into here.
</p>
<blockquote class="citation">
<ul><li>With this ticket, <code>GroupAlgebra(SymmetricGroup(3),QQ)</code> now properly
constructs an instance of <code>SymmetricGroupAlgebra(3,Q)</code>. Given that
<code>SymmetricGroupAlgebra</code> has a custom <code>_repr_</code>, this induces quite a
few trivial doctest updates, from:
<pre class="wiki"> Group algebra of Symmetric group of order 3! as a permutation group over Rational Field
</pre>to:
<pre class="wiki"> Symmetric group algebra of order 3 over Rational Field
</pre></li></ul><blockquote>
<p>
While we are at it, I was wondering whether we did not want to use
the occasion to remove that custom repr for consistency, especially
since the that custom repr hides which specific implementation of
the symmetric group algebra it used:
</p>
<pre class="wiki"> sage: SymmetricGroupAlgebra(QQ, WeylGroup(["A",3]))
Symmetric group algebra of order 4 over Rational Field
</pre></blockquote>
</blockquote>
<p>
I like the compactness of the current representation. Plus the actual group that is used I see of as more of an implementation detail.
</p>
<blockquote class="citation">
<blockquote>
<p>
Maybe we would want to also change the generic repr to produce:
</p>
<pre class="wiki"> XXX group algebra over QQ
</pre><p>
instead of
</p>
<pre class="wiki"> Group algebra of XXX group over QQ
</pre></blockquote>
<blockquote>
<p>
Opinions?
</p>
</blockquote>
</blockquote>
<p>
I think the latter is better, but it is slightly redundant. I would say
</p>
<pre class="wiki">Algebra of XXX over YYY
</pre><p>
as it is more compatible when we expand to, e.g., monoids and it contains enough information to explain the object. Although it
</p>
<blockquote class="citation">
<ul><li>We have been struck once again in this ticket by the dreaded
category MRO issue. I have one commit in <code>sage.misc.c3_controlled</code>
which makes things more robust in waiting for a proper
implementation of <a class="new ticket" href="https://trac.sagemath.org/ticket/22962" title="enhancement: Toward singleton categories: subcategories of Modules (new)">#22962</a>.
</li></ul><blockquote>
<p>
Should I move this commit to a separate ticket?
</p>
</blockquote>
</blockquote>
<p>
Yes, please. I don't understand this change and having an explanation for future reference would be beneficial.
</p>
<blockquote class="citation">
<ul><li>Lifting <code>Parent.construction</code> to <code>Sets.ParentMethods</code> forced me to
fix a couple parents to properly initialize their categories (that's
a good thing!). Should I move this, and the lifting of
"construction", to a separate ticket?
</li></ul></blockquote>
<p>
Probably; it is good practice.
</p>
TickettscrimSat, 13 May 2017 17:14:28 GMT
https://trac.sagemath.org/ticket/18700#comment:48
https://trac.sagemath.org/ticket/18700#comment:48
<p>
Looking at the current code, I really do not like the hack in <code>_coerce_map_from_</code>. IMO, we should never have a <code>_coerce_map_from_</code> in the category as any such coercions should be defined using functors. If you need to have <code>_coerce_map_from_</code>, then we need a concrete class as you are doing something that is implementation specific.
</p>
TicketnthierySat, 13 May 2017 21:50:07 GMT
https://trac.sagemath.org/ticket/18700#comment:49
https://trac.sagemath.org/ticket/18700#comment:49
<p>
Hi Travis,
</p>
<p>
Thanks for the feedback.
</p>
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/18700#comment:48" title="Comment 48">tscrim</a>:
</p>
<blockquote class="citation">
<p>
Looking at the current code, I really do not like the hack in <code>_coerce_map_from_</code>. IMO, we should never have a <code>_coerce_map_from_</code> in the category as any such coercions should be defined using functors. If you need to have <code>_coerce_map_from_</code>, then we need a concrete class as you are doing something that is implementation specific.
</p>
</blockquote>
<p>
I agree that a conversion in a category would be dubious. But here we are speaking about coercions that implement canonical embeddings.
</p>
<p>
Look at the coercion from the base ring into a unital algebra. There is nothing implementation-specific in it. It's all about mathematics, and it belongs to the category of unital algebras.
</p>
<p>
Here again it's about the mathematical fact that "the embedding of G into G' lifts to an embedding of KG into KG'"; there is no need to know the specifics of the data structure for KG' to implement that fact. Therefore I see no reason why some concrete class should be involved.
It just turns out that our functor is actually a functorial construction and requires a "split implementation" (see <a class="ext-link" href="http://opendreamkit.org/2016/08/01/CICM/#terminology"><span class="icon"></span>http://opendreamkit.org/2016/08/01/CICM/#terminology</a> by lack of better terminology). Hence its implementation ends up being in the functorial construction categories.
</p>
<p>
Cheers,
</p>
<blockquote>
<p>
Nicolas
</p>
</blockquote>
TickettscrimSat, 13 May 2017 22:31:42 GMT
https://trac.sagemath.org/ticket/18700#comment:50
https://trac.sagemath.org/ticket/18700#comment:50
<p>
I didn't say anything about conversions, which is even worse. Yet, I agree that the mathematics are there, but you have a specific implementation of <code>_coerce_map_from_</code> in the category. That is something that should be handled by the construction functor. The other place that we could do this is register a coercion in the <code>__init_extra__</code>. The last option would be to lift <code>_coerce_map_from_</code> from <code>Parent</code> to <code>Sets.ParentMethods</code>, but I want to keep an idiom for concrete implementations.
</p>
TicketnthierySun, 14 May 2017 02:06:31 GMT
https://trac.sagemath.org/ticket/18700#comment:51
https://trac.sagemath.org/ticket/18700#comment:51
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/18700#comment:50" title="Comment 50">tscrim</a>:
</p>
<blockquote class="citation">
<p>
The other place that we could do this is register a coercion in the <code>__init_extra__</code>.
</p>
</blockquote>
<p>
Alas, this does not work: there are many K'[G'] that could coerce in
K[G], and we can't explicitly register conversions for all of them.
</p>
<p>
I have been dreaming for a long time (already in MuPAD-Combinat) of
having a way to register a single "conversion pattern" that would
later get lazily instantiated as needed. <code>_coerce_map_from</code> is kind of
doing that job.
</p>
<blockquote class="citation">
<p>
but you have a specific implementation of <code>_coerce_map_from_</code> in the
category. The last option would be to lift <code>_coerce_map_from_</code> from
<code>Parent</code> to <code>Sets.ParentMethods</code>, but I want to keep an idiom for
concrete implementations.
</p>
</blockquote>
<p>
Hmm, are you worried that, if we implement a <code>_coerce_map_from</code> in
some category C, then we are forcing each of its concrete parent to
make a choice between either restraining itself from implementing a
custom <code>_coerce_map_from</code> method, or not benefiting from the generic
coercions provided by C?
</p>
<p>
If yes, I believe we are fine: both can implement <code>_coerce_map_from</code>
methods as long as they play cooperatively with super calls. That's
what I have done here.
</p>
<p>
It's not yet common practice. However if, as I would tend to foresee,
it becomes a common situations to have both a parent and several of
its categories implement <code>_coerce_map_from</code> methods, we could better
support this idiom by:
</p>
<ul><li>Changing Parent's default implementation to do a super call
</li></ul><ul><li>Encourage if not enforce in the documentation that any
implementation of <code>_coerce_map_from</code> should end with a super call.
</li></ul><p>
Granted, super calls within the categories are somewhat inconvenient
due to recovering the relevant class; it should get better with
singleton categories.
</p>
<p>
Cheers,
</p>
<blockquote>
<p>
Nicolas
</p>
</blockquote>
TicketnthieryMon, 15 May 2017 03:24:47 GMT
https://trac.sagemath.org/ticket/18700#comment:52
https://trac.sagemath.org/ticket/18700#comment:52
<p>
See <a class="closed ticket" href="https://trac.sagemath.org/ticket/23000" title="defect: Fix inconsistency in `Modules.FiniteDimensional.extra_super_categories` (closed: fixed)">#23000</a>
</p>
TickettscrimMon, 15 May 2017 16:26:51 GMTdependencies set
https://trac.sagemath.org/ticket/18700#comment:53
https://trac.sagemath.org/ticket/18700#comment:53
<ul>
<li><strong>dependencies</strong>
set to <em>#23000</em>
</li>
</ul>
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/18700#comment:51" title="Comment 51">nthiery</a>:
</p>
<blockquote class="citation">
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/18700#comment:50" title="Comment 50">tscrim</a>:
</p>
<blockquote class="citation">
<p>
The other place that we could do this is register a coercion in the <code>__init_extra__</code>.
</p>
</blockquote>
<p>
Alas, this does not work: there are many K'[G'] that could coerce in
K[G], and we can't explicitly register conversions for all of them.
</p>
<p>
I have been dreaming for a long time (already in MuPAD-Combinat) of
having a way to register a single "conversion pattern" that would
later get lazily instantiated as needed. <code>_coerce_map_from</code> is kind of
doing that job.
</p>
</blockquote>
<p>
Yes and no. My strong opinion is that when you have a <code>construction</code> that returns a <code>ConstructionFunctor</code>, then that functor should be able to set. This is both a natural place and should be simple to implement. Unfortunately, we don't have such a hook yet. I would much rather have a concrete class with a <code>_coerce_map_from_</code> in the meantime than the hack around as a (lightweight) concrete class is easier to get rid of in the future.
</p>
<blockquote class="citation">
<blockquote class="citation">
<p>
but you have a specific implementation of <code>_coerce_map_from_</code> in the
category. The last option would be to lift <code>_coerce_map_from_</code> from
<code>Parent</code> to <code>Sets.ParentMethods</code>, but I want to keep an idiom for
concrete implementations.
</p>
</blockquote>
<p>
Hmm, are you worried that, if we implement a <code>_coerce_map_from</code> in
some category C, then we are forcing each of its concrete parent to
make a choice between either restraining itself from implementing a
custom <code>_coerce_map_from</code> method, or not benefiting from the generic
coercions provided by C?
</p>
<p>
If yes, I believe we are fine: both can implement <code>_coerce_map_from</code>
methods as long as they play cooperatively with super calls. That's
what I have done here.
</p>
</blockquote>
<p>
That is part of my concern, as it can require some finesse to get around it as you have done. However, that is not my biggest worry (see below).
</p>
<blockquote class="citation">
<p>
It's not yet common practice. However if, as I would tend to foresee,
it becomes a common situations to have both a parent and several of
its categories implement <code>_coerce_map_from</code> methods, we could better
support this idiom by:
</p>
<ul><li>Changing Parent's default implementation to do a super call
</li></ul><ul><li>Encourage if not enforce in the documentation that any
implementation of <code>_coerce_map_from</code> should end with a super call.
</li></ul></blockquote>
<p>
I do not think this is a good idea to recommend super calls on <code>_coerce_map_from_</code> as they could go to a category with less structure, allowing more permissive coercions (e.g., graded algebras to algebras). I also think that <code>_coerce_map_from_</code> should be reserved for concrete implementations, where specific information about the data structure can be used for a similar reason.
</p>
<blockquote class="citation">
<p>
Granted, super calls within the categories are somewhat inconvenient
due to recovering the relevant class; it should get better with
singleton categories.
</p>
</blockquote>
<p>
I am not completely convinced of this, as it should be determined by the hierarchy of the categories. I think the biggest technical limitation is the Python2 version of <code>super</code>.
</p>
TicketnthieryTue, 16 May 2017 02:48:00 GMT
https://trac.sagemath.org/ticket/18700#comment:54
https://trac.sagemath.org/ticket/18700#comment:54
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/18700#comment:53" title="Comment 53">tscrim</a>:
</p>
<blockquote class="citation">
<p>
Yes and no. My strong opinion is that when you have a <code>construction</code> that returns a <code>ConstructionFunctor</code>, then that functor should be able to set. This is both a natural place and should be simple to implement. Unfortunately, we don't have such a hook yet. I would much rather have a concrete class with a <code>_coerce_map_from_</code> in the meantime than the hack around as a (lightweight) concrete class is easier to get rid of in the future.
</p>
</blockquote>
<p>
A lightweight concrete class also has its downside: Let's say Alice
wants to implement a monoid algebra A, but using a different data
structure than Combinatorial Free Module. Maybe someone wanting to
endow the existing polynomial rings with their natural monoid algebra
structure. If there is a lightweight concrete class C, then Alice is
forced to chose to either have A inherit from C (which may conflict
with the existing data structure), or to not benefit from the features
of C.
</p>
<p>
For cartesian products, there already are at least two concrete
implementations.
</p>
<p>
As for the "hack around": maybe the infrastructure could streamline
further the process, but I do not see it as a hack: there are several
players that together provide the features of group algebras, and this
is just making them cooperate also on the coercion definitions.
</p>
<blockquote class="citation">
<p>
I do not think this is a good idea to recommend super calls on
<code>_coerce_map_from_</code> as they could go to a category with less
structure, allowing more permissive coercions (e.g., graded algebras
to algebras).
</p>
</blockquote>
<p>
There is a point here: Sage has always been fuzzy about its definition
of coercions which should be "morphisms for whatever the structure the
objects could have", and we are hitting on that fuzz. I believe this
should be in the contract: "by declaring yourself in that category
(especially a functorial construction category), you accept that there
will be a coercion from XXX".
</p>
<p>
My approach to that has always be to minimize the number of coercions
defined by default :-)
</p>
<blockquote class="citation">
<p>
I also think that <code>_coerce_map_from_</code> should be reserved for
concrete implementations, where specific information about the data
structure can be used for a similar reason.
</p>
</blockquote>
<p>
Good point: a concrete implementation may want to exploit the internal
data structure to provide a more efficient implementation of the
morphism. Hence we should separate the implementation of the morphism
from its declaration as coercion, so that the implementation can be
overridden in subclass. That's what we already do for
"from_base_ring", and did in similar situations in MuPAD-Combinat.
I'll implement that here.
</p>
<blockquote class="citation">
<p>
I am not completely convinced of this, as it should be determined by
the hierarchy of the categories. I think the biggest technical
limitation is the Python2 version of <code>super</code>.
</p>
</blockquote>
<p>
Quite true! I guess I am just not yet in the frame of mind "Python 3
will come soon enough that I can start dreaming about all the good use
we could make of its features" :-) I am not sure though Python3 with
help with super and categories for Cython classes. Let's see.
</p>
<p>
Cheers,
</p>
<blockquote>
<p>
Nicolas
</p>
</blockquote>
TickettscrimTue, 16 May 2017 04:54:39 GMT
https://trac.sagemath.org/ticket/18700#comment:55
https://trac.sagemath.org/ticket/18700#comment:55
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/18700#comment:54" title="Comment 54">nthiery</a>:
</p>
<blockquote class="citation">
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/18700#comment:53" title="Comment 53">tscrim</a>:
A lightweight concrete class also has its downside: Let's say Alice
wants to implement a monoid algebra A, but using a different data
structure than Combinatorial Free Module. Maybe someone wanting to
endow the existing polynomial rings with their natural monoid algebra
structure. If there is a lightweight concrete class C, then Alice is
forced to chose to either have A inherit from C (which may conflict
with the existing data structure), or to not benefit from the features
of C.
</p>
</blockquote>
<p>
That feels like a fallacy to me. I am saying we keep the current concrete implementation as light as possible as a placeholder until we put the hook in the correct place.
</p>
<blockquote class="citation">
<p>
As for the "hack around": maybe the infrastructure could streamline
further the process, but I do not see it as a hack: there are several
players that together provide the features of group algebras, and this
is just making them cooperate also on the coercion definitions.
</p>
</blockquote>
<p>
You're circumventing the MRO with the second call to <code>_coerce_map_from_</code>, which has to possible catch an <code>AttributeError</code>. So if you want to do this at the category level, then you should define the generic call at that category level. However, this could have speed implications as it is a <code>cpdef</code> method currently, in addition to the other problems we are discussing.
</p>
<blockquote class="citation">
<blockquote class="citation">
<p>
I do not think this is a good idea to recommend super calls on
<code>_coerce_map_from_</code> as they could go to a category with less
structure, allowing more permissive coercions (e.g., graded algebras
to algebras).
</p>
</blockquote>
<p>
There is a point here: Sage has always been fuzzy about its definition
of coercions which should be "morphisms for whatever the structure the
objects could have", and we are hitting on that fuzz. I believe this
should be in the contract: "by declaring yourself in that category
(especially a functorial construction category), you accept that there
will be a coercion from XXX".
</p>
</blockquote>
<p>
I feel like that is a something that would come from registering coercions using a <code>ConstructionFunctor</code> and making that the default result of <code>construction</code>. However, don't think we should have that be (one of) the defining condition of a coercion as it is too strong, but instead a condition on objects the category, as your statement makes.
</p>
TicketgitWed, 17 May 2017 04:20:44 GMTcommit changed
https://trac.sagemath.org/ticket/18700#comment:56
https://trac.sagemath.org/ticket/18700#comment:56
<ul>
<li><strong>commit</strong>
changed from <em>045367eeb1323944891aa78de1eecb3ec9b443cd</em> to <em>b462163f82c48c622be1b10993e21de5b881c405</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="https://git.sagemath.org/sage.git/commit/?id=1d1a406c526b392dab8e50f9b3519fbb12d7c709"><span class="icon"></span>1d1a406</a></td><td><code>18700: let Parent and Module cooperate with super classes for coercion lookup with _coerce_map_from_</code>
</td></tr><tr><td><a class="ext-link" href="https://git.sagemath.org/sage.git/commit/?id=210e9458a536c3e3d9f52b1ac8489ae68d7b7b83"><span class="icon"></span>210e945</a></td><td><code>18700: lift all features of the GroupAlgebra class to the algebra functor and categories + doc</code>
</td></tr><tr><td><a class="ext-link" href="https://git.sagemath.org/sage.git/commit/?id=588e9ee9b5be6cebc988d680877620889a9cd5e1"><span class="icon"></span>588e9ee</a></td><td><code>18700: lift CombinatorialFreeModule.gens to Modules.WithBasis.FiniteDimensional</code>
</td></tr><tr><td><a class="ext-link" href="https://git.sagemath.org/sage.git/commit/?id=7bb569a7419a442ddf9a8920590bdb3d65b63870"><span class="icon"></span>7bb569a</a></td><td><code>18700: conversion from formal sums to CombinatorialFreeModule</code>
</td></tr><tr><td><a class="ext-link" href="https://git.sagemath.org/sage.git/commit/?id=d6441b0522f85032dd2c7e48e759651739686e6c"><span class="icon"></span>d6441b0</a></td><td><code>18700: Implement Permutations.degree, for consistency with SymmetricGroup</code>
</td></tr><tr><td><a class="ext-link" href="https://git.sagemath.org/sage.git/commit/?id=b8be30fb8de29f143b7a02a0294d0b5f477dc3ef"><span class="icon"></span>b8be30f</a></td><td><code>18700: Implement PermutationGroup.ngens, for consistency with its gen(i), ngens, ...</code>
</td></tr><tr><td><a class="ext-link" href="https://git.sagemath.org/sage.git/commit/?id=88f5c8451830914196a9ff556c085baae2b8b4e4"><span class="icon"></span>88f5c84</a></td><td><code>18700: Symmetric group algebra: better support for SymmetricGroupAlgebra(0, QQ)</code>
</td></tr><tr><td><a class="ext-link" href="https://git.sagemath.org/sage.git/commit/?id=63a2bba7359448b1c3639d41c4d8554251aba6f3"><span class="icon"></span>63a2bba</a></td><td><code>18700: lift group-algebra related methods up the categories</code>
</td></tr><tr><td><a class="ext-link" href="https://git.sagemath.org/sage.git/commit/?id=e2ab56f1bb5d9fc33638dc53281b113337577ee1"><span class="icon"></span>e2ab56f</a></td><td><code>18700: follow up on getting rid of GroupAlgebra + documentation cleanup from previous commit</code>
</td></tr><tr><td><a class="ext-link" href="https://git.sagemath.org/sage.git/commit/?id=b462163f82c48c622be1b10993e21de5b881c405"><span class="icon"></span>b462163</a></td><td><code>18700: trivial documentation updates</code>
</td></tr></table>
TicketnthieryWed, 17 May 2017 04:23:17 GMT
https://trac.sagemath.org/ticket/18700#comment:57
https://trac.sagemath.org/ticket/18700#comment:57
<p>
With that current version, all tests pass, and the commits are reasonably clean.
</p>
<p>
Now it's way time to go to bed. I'll discuss here tomorrow the state of affair, and in particular which comments I have taken into account, or not, or not yet.
</p>
<p>
Cheers,
</p>
TicketnthieryFri, 19 May 2017 20:46:01 GMT
https://trac.sagemath.org/ticket/18700#comment:58
https://trac.sagemath.org/ticket/18700#comment:58
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/18700#comment:47" title="Comment 47">tscrim</a>:
</p>
<blockquote class="citation">
<blockquote class="citation">
<p>
... About documentation ...
</p>
</blockquote>
<p>
Well, none of this will be available from <code>G?</code>, so I would put it in
<code>GroupAlgebra</code>. However, there are two slightly different topics.
The first would be basic usage and properties, the other would be
provide doctests of functionality. What used to be in the
module-level doc of <code>algebras/group_algebra.py</code> is the latter. For
this, a natural place is where we provide the majority of the
implementation: the <code>Groups.Algebras</code> category. In
<code>Sets.ParentMethods.algebra</code>, we should give the necessary
information to construct the object as it is a generic method.
</p>
</blockquote>
<p>
Right, it's indeed a shame that <code>G?</code> did not bring anything useful. I
fixed this by setting its <code>__doc__</code> attribute in
<code>Sets.ParentMethods.algebra</code>.
</p>
<p>
After moving things around while merging all the pieces together, I
ended up adopting the following plan:
</p>
<ul><li>Have the module <code>sage.categories.algebra_functor</code> start with a
tutorial on "group algebras and generalizations", introducing as its
name suggests the topic with group algebras and then generalizing.
</li></ul><ul><li>Link this tutorial from the thematic tutorials
</li></ul><ul><li>Proceed after the tutorial with a test section containing all tests
that did not have a natural location elsewhere (e.g. in the
docstring of a method).
</li></ul><ul><li>For each of <code>Sets.ParentMethods.algebra</code>, <code>GroupAlgebra</code>,
<code>GroupAlgebras</code> and the group algebra functor, have a short doc with
the minimum definition, examples and specifications that are
specific to each, and a proeminent link to the tutorial.
</li></ul><blockquote class="citation">
<p>
IMO, it is better to have the history where you make the first
attempt in case there is something we might want to salvage from it
later on down the road.
</p>
</blockquote>
<p>
Good point. I kept the history as is for your first implementation and
only rewrote history for my own changes (no intermediate point was of
particular value).
</p>
<blockquote class="citation">
<blockquote class="citation">
<p>
... about repr ...
</p>
</blockquote>
<p>
I would say
</p>
<pre class="wiki">Algebra of XXX over YYY
</pre><p>
as it is more compatible when we expand to, e.g., monoids and it contains enough information to explain the object.
</p>
</blockquote>
<p>
I like it as well. Done.
</p>
<blockquote class="citation">
<blockquote class="citation">
<blockquote>
<p>
about moving some stuff to separate tickets.
</p>
</blockquote>
</blockquote>
<p>
Yes, please / probably; it is good practice.
</p>
</blockquote>
<p>
Done.
</p>
TickettscrimFri, 19 May 2017 20:52:13 GMT
https://trac.sagemath.org/ticket/18700#comment:59
https://trac.sagemath.org/ticket/18700#comment:59
<p>
Sounds like a good plan.
</p>
TicketnthieryFri, 19 May 2017 21:23:21 GMT
https://trac.sagemath.org/ticket/18700#comment:60
https://trac.sagemath.org/ticket/18700#comment:60
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/18700#comment:55" title="Comment 55">tscrim</a>:
</p>
<blockquote class="citation">
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/18700#comment:54" title="Comment 54">nthiery</a>:
</p>
<blockquote class="citation">
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/18700#comment:53" title="Comment 53">tscrim</a>:
A lightweight concrete class also has its downside: Let's say Alice
wants to implement a monoid algebra A, but using a different data
structure than Combinatorial Free Module. Maybe someone wanting to
endow the existing polynomial rings with their natural monoid algebra
structure. If there is a lightweight concrete class C, then Alice is
forced to chose to either have A inherit from C (which may conflict
with the existing data structure), or to not benefit from the features
of C.
</p>
</blockquote>
<p>
That feels like a fallacy to me.
</p>
</blockquote>
<p>
In my world it's a genuine comment originating from being struck by
similar situations several times :-)
</p>
<blockquote class="citation">
<blockquote class="citation">
<p>
As for the "hack around": maybe the infrastructure could streamline
further the process, but I do not see it as a hack: there are several
players that together provide the features of group algebras, and this
is just making them cooperate also on the coercion definitions.
</p>
</blockquote>
<p>
You're circumventing the MRO with the second call to <code>_coerce_map_from_</code>, which has to possible catch an <code>AttributeError</code>. So if you want to do this at the category level, then you should define the generic call at that category level. However, this could have speed implications as it is a <code>cpdef</code> method currently, in addition to the other problems we are discussing.
</p>
</blockquote>
<p>
Oh, I see: I had forgotten I had not finished the work: the code back
then was indeed a hack in CFM to circumvent the implementation in
<code>Module</code>. I have reworked this so that the default implementation
really is in <code>Sets.ParentMethods</code>, and both <code>Module</code> and <code>Parent</code> play
cooperatively by ending with a super call instead of returning <code>None</code>
if they have nothing to offer.
</p>
<p>
This does not close the discussion on whether we want
<code>_coerce_map_from_</code> play cooperatively along the hierarchy of classes.
But if we do, the above implementation sounds clean to me. Having the
cpdef <code>_coerce_map_from_</code> in <code>Parent</code> enables subclasses to implement
cpdef <code>_coerce_map_from_</code> that are fast when they succeed, which I
believe is the most important.
</p>
<p>
It would be nice to have the coercion be tied to the construction
functor. And incidentally it would be nice to have a bivariate functor
rather than our two "curried" univariate ones (<code>GroupAlgebra(G)(Q)</code>
and <code>AlgebraFunctor(Q)(G)</code>).
</p>
<p>
In the mean time, I aiming right now for the concrete-class-less
approach, in particular because the later transition will be
transparent to subclasses.
</p>
<p>
Cheers,
</p>
<blockquote>
<p>
Nicolas
</p>
</blockquote>
TickettscrimSat, 20 May 2017 17:22:24 GMT
https://trac.sagemath.org/ticket/18700#comment:61
https://trac.sagemath.org/ticket/18700#comment:61
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/18700#comment:60" title="Comment 60">nthiery</a>:
</p>
<blockquote class="citation">
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/18700#comment:55" title="Comment 55">tscrim</a>:
</p>
<blockquote class="citation">
<p>
That feels like a fallacy to me.
</p>
</blockquote>
<p>
In my world it's a genuine comment originating from being struck by
similar situations several times :-)
</p>
</blockquote>
<p>
I know we've come across it in the past, but the fallacy comes from the fact it might be a problem in the future and that there are other solutions (e.g., code duplication, which wouldn't be much since it is lightweight). Plus, this is only meant to be there short-term while we implement a more general framework.
</p>
<blockquote class="citation">
<p>
Oh, I see: I had forgotten I had not finished the work: the code back
then was indeed a hack in CFM to circumvent the implementation in
<code>Module</code>. I have reworked this so that the default implementation
really is in <code>Sets.ParentMethods</code>, and both <code>Module</code> and <code>Parent</code> play
cooperatively by ending with a super call instead of returning <code>None</code>
if they have nothing to offer.
</p>
</blockquote>
<p>
Yes, this is much better.
</p>
<blockquote class="citation">
<p>
This does not close the discussion on whether we want
<code>_coerce_map_from_</code> play cooperatively along the hierarchy of classes.
But if we do, the above implementation sounds clean to me. Having the
cpdef <code>_coerce_map_from_</code> in <code>Parent</code> enables subclasses to implement
cpdef <code>_coerce_map_from_</code> that are fast when they succeed, which I
believe is the most important.
</p>
</blockquote>
<p>
I don't like this as it means coercion testing has an extra level of an MRO lookup plus a python call. So it ends up adding a bit of extra overhead. Granted, this is not a huge difference as coercions are cached, and this is rarely called (relatively speaking). So it is not a strong argument. However, I'm still very worried about incidentally allowing coercions between weakened structures by enforcing <code>_coerce_map_from_</code> to make super calls. Plus there are the technical limitations.
</p>
<p>
However, we need a better mechanism for putting category level information into the coercion framework as this is essentially duplicating information (in a private method) that would be available from the functor.
</p>
<blockquote class="citation">
<p>
It would be nice to have the coercion be tied to the construction
functor.
</p>
</blockquote>
<p>
I can do this on a followup ticket. At least I have a good idea about how this should be done. Although, thinking about it, it might have a similar sort of slowdown as lifting up the <code>_coerce_map_from_</code>. However, it is something that we should do IMO.
</p>
<blockquote class="citation">
<p>
And incidentally it would be nice to have a bivariate functor
rather than our two "curried" univariate ones (<code>GroupAlgebra(G)(Q)</code>
and <code>AlgebraFunctor(Q)(G)</code>).
</p>
</blockquote>
<p>
There is a problem with resolving this due to ambiguities with the codomain category. One possibility is that we have a category with either a single object (up to isomorphism), the group algebra <code>R[G]</code> for a fixed group <code>G</code> and ring <code>R</code>. Another possibility is that we allow the ring <code>R</code> to vary. The last is we allow <code>G</code> to vary. The former is either the single object category or allows <code>R</code> to vary. The latter is either the single object category or allows <code>G</code> to vary.
</p>
<p>
Part of this might come from current technical limitations, i.e., <code>CovariantFunctorialConstruction</code> versus <code>ConstructionFunctor</code>. I would probably say that by moving so much up to the category, we have made this problem worse to almost forcing the category to have a single object, which is not what we want.
</p>
<blockquote class="citation">
<p>
In the mean time, I aiming right now for the concrete-class-less
approach, in particular because the later transition will be
transparent to subclasses.
</p>
</blockquote>
<p>
I still have my strong reservations with not allowing a concrete class (at this point) in general. I am willing to bend very far, but not all the way at this point. As a compromise, could we have the lightweight concrete class for now until we implement a better infrastructure for the coercion maps? It does not make anything worse (from your POV) than what we currently have.
</p>
TicketgitThu, 25 May 2017 15:12:49 GMTcommit changed
https://trac.sagemath.org/ticket/18700#comment:62
https://trac.sagemath.org/ticket/18700#comment:62
<ul>
<li><strong>commit</strong>
changed from <em>b462163f82c48c622be1b10993e21de5b881c405</em> to <em>a1431e0256ed3eb1071e0ddeda1497810be7efdc</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1. New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="https://git.sagemath.org/sage.git/commit/?id=a1431e0256ed3eb1071e0ddeda1497810be7efdc"><span class="icon"></span>a1431e0</a></td><td><code>18700: fix generalization of Modules.WithBasis.random_element</code>
</td></tr></table>
TicketnthieryWed, 14 Jun 2017 02:56:38 GMT
https://trac.sagemath.org/ticket/18700#comment:63
https://trac.sagemath.org/ticket/18700#comment:63
<p>
Brief summary of the phone call we had together with Travis today (Thanks Travis for relaunching the discussion!): this ticket is basically ready to go (up to rebasing, and possibly fixing a few more doctests). We did not come to an agreement on whether to use a concrete class or not. I still believe that using one instead of enabling <code>coerce_map_from</code> in the categories is limiting the flexibility without a compelling safety reason.
</p>
<p>
That being said, we need to move on and the design decision is fairly easy to revert. So I gave my green light for switching back to a very minimal concrete class whose role is only to handle the coercions.
</p>
<p>
I asked Travis to create a ticket with the current branch to keep the code around in case later insights or use cases makes us rethink the decision.
</p>
TicketgitFri, 16 Jun 2017 02:37:05 GMTcommit changed
https://trac.sagemath.org/ticket/18700#comment:64
https://trac.sagemath.org/ticket/18700#comment:64
<ul>
<li><strong>commit</strong>
changed from <em>a1431e0256ed3eb1071e0ddeda1497810be7efdc</em> to <em>64e9cd0f7aad5509ad47dad542f238eac9a119c9</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1. New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="https://git.sagemath.org/sage.git/commit/?id=e4f7d47d2a7c8198b5d8829dc231d47e4decdbbb"><span class="icon"></span>e4f7d47</a></td><td><code>Merge branch 'public/groups/standardize_group_algebras-18700' of git://trac.sagemath.org/sage into public/groups/standardize_group_algebras-18700</code>
</td></tr><tr><td><a class="ext-link" href="https://git.sagemath.org/sage.git/commit/?id=64e9cd0f7aad5509ad47dad542f238eac9a119c9"><span class="icon"></span>64e9cd0</a></td><td><code>Removing _coerce_map_from_ from the category implementation.</code>
</td></tr></table>
TickettscrimFri, 16 Jun 2017 02:42:38 GMTstatus changed
https://trac.sagemath.org/ticket/18700#comment:65
https://trac.sagemath.org/ticket/18700#comment:65
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_review</em>
</li>
</ul>
<p>
I've done the partial revert. The previous code is on <a class="new ticket" href="https://trac.sagemath.org/ticket/23252" title="enhancement: Implement coercions in the category framework (new)">#23252</a>. Needs double check. More comments likely to follow later.
</p>
TickettscrimFri, 16 Jun 2017 13:03:07 GMTauthor changed
https://trac.sagemath.org/ticket/18700#comment:66
https://trac.sagemath.org/ticket/18700#comment:66
<ul>
<li><strong>author</strong>
changed from <em>Travis Scrimshaw</em> to <em>TravisScrimshaw, Nicolas M. Thiéry</em>
</li>
</ul>
<p>
Thank you for the productive discussion(s) Nicolas.
</p>
<p>
Okay, now I have a little more time for a detailed reply. I've added back in a small concrete class to handle the coercion until we have a better mechanism for coercion at the level of categories. However, we can continue that discussion on <a class="new ticket" href="https://trac.sagemath.org/ticket/23252" title="enhancement: Implement coercions in the category framework (new)">#23252</a>. I also scrubbed the <code>_coerce_map_from_</code> calls up through the category as I'm still not convinced this is the way forward.
</p>
<p>
The version before my changes is on that ticket, which I rebased to <code>8.0.beta10</code> as a way to try and ensure they won't accidentally get overwritten by my last commit. If someone could just double-check my changes, then we can set a positive review.
</p>
TicketgitFri, 16 Jun 2017 22:50:32 GMTcommit changed
https://trac.sagemath.org/ticket/18700#comment:67
https://trac.sagemath.org/ticket/18700#comment:67
<ul>
<li><strong>commit</strong>
changed from <em>64e9cd0f7aad5509ad47dad542f238eac9a119c9</em> to <em>4e9200251bfe9c2828d22cb4337b4984ef188bb6</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1. New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="https://git.sagemath.org/sage.git/commit/?id=4e9200251bfe9c2828d22cb4337b4984ef188bb6"><span class="icon"></span>4e92002</a></td><td><code>Merge branch 'develop' into t/18700/public/groups/standardize_group_algebras-18700</code>
</td></tr></table>
Ticketbsalisbury1Fri, 16 Jun 2017 22:51:22 GMT
https://trac.sagemath.org/ticket/18700#comment:68
https://trac.sagemath.org/ticket/18700#comment:68
<p>
I merged the latest develop branch. All tests passed but documentation doesn't build.
</p>
TicketgitSat, 17 Jun 2017 05:12:53 GMTcommit changed
https://trac.sagemath.org/ticket/18700#comment:69
https://trac.sagemath.org/ticket/18700#comment:69
<ul>
<li><strong>commit</strong>
changed from <em>4e9200251bfe9c2828d22cb4337b4984ef188bb6</em> to <em>c2e1c053fcd363569ca0ec6d7e781a61e1445031</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1. New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="https://git.sagemath.org/sage.git/commit/?id=1586d6546eca1ef50538185b3f05019e0008a948"><span class="icon"></span>1586d65</a></td><td><code>Merge branch 'develop' into public/groups/standardize_group_algebras-18700</code>
</td></tr><tr><td><a class="ext-link" href="https://git.sagemath.org/sage.git/commit/?id=e6ef3c10a54265e71b674431be5f164a9a66b02a"><span class="icon"></span>e6ef3c1</a></td><td><code>Doc fixes.</code>
</td></tr><tr><td><a class="ext-link" href="https://git.sagemath.org/sage.git/commit/?id=c2e1c053fcd363569ca0ec6d7e781a61e1445031"><span class="icon"></span>c2e1c05</a></td><td><code>Merge branch 'public/groups/standardize_group_algebras-18700' of git://trac.sagemath.org/sage into public/groups/standardize_group_algebras-18700</code>
</td></tr></table>
TickettscrimSat, 17 Jun 2017 05:14:28 GMT
https://trac.sagemath.org/ticket/18700#comment:70
https://trac.sagemath.org/ticket/18700#comment:70
<p>
This fixes the docbuild issues for me (plus a few other misc doc issues I noticed while trying to fix it).
</p>
Ticketbsalisbury1Sun, 18 Jun 2017 16:28:22 GMTstatus changed
https://trac.sagemath.org/ticket/18700#comment:71
https://trac.sagemath.org/ticket/18700#comment:71
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>positive_review</em>
</li>
</ul>
<p>
Looks good now. Thanks!
</p>
TickettscrimWed, 21 Jun 2017 01:46:23 GMTauthor changed
https://trac.sagemath.org/ticket/18700#comment:72
https://trac.sagemath.org/ticket/18700#comment:72
<ul>
<li><strong>author</strong>
changed from <em>TravisScrimshaw, Nicolas M. Thiéry</em> to <em>Travis Scrimshaw, Nicolas M. Thiéry</em>
</li>
</ul>
TicketvbraunWed, 21 Jun 2017 21:20:05 GMT
https://trac.sagemath.org/ticket/18700#comment:73
https://trac.sagemath.org/ticket/18700#comment:73
<p>
Search for <code>algerbas</code> in the patch, obvious typo. There should be a test that catches that
</p>
<pre class="wiki">sage -t --long --warn-long 67.9 src/sage/algebras/group_algebra.py
**********************************************************************
File "src/sage/algebras/group_algebra.py", line 138, in sage.algebras.group_algebra.GroupAlgebra_class._coerce_map_from_
Failed example:
ZG.coerce_map_from(H)
Expected:
Conversion map:
From: Cyclic group of order 3 as a permutation group
To: Algebra of Dihedral group of order 6 as a permutation group over Integer Ring
Got:
Coercion map:
From: Cyclic group of order 3 as a permutation group
To: Algebra of Dihedral group of order 6 as a permutation group over Integer Ring
</pre>
TicketvbraunWed, 21 Jun 2017 21:20:17 GMTstatus changed
https://trac.sagemath.org/ticket/18700#comment:74
https://trac.sagemath.org/ticket/18700#comment:74
<ul>
<li><strong>status</strong>
changed from <em>positive_review</em> to <em>needs_work</em>
</li>
</ul>
TicketgitWed, 21 Jun 2017 23:29:00 GMTcommit changed
https://trac.sagemath.org/ticket/18700#comment:75
https://trac.sagemath.org/ticket/18700#comment:75
<ul>
<li><strong>commit</strong>
changed from <em>c2e1c053fcd363569ca0ec6d7e781a61e1445031</em> to <em>a93b1e9f5b3f989c67e433e043b616073861502e</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1. New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="https://git.sagemath.org/sage.git/commit/?id=6be53f3fbc11bea3ff91918d539085b52e262cca"><span class="icon"></span>6be53f3</a></td><td><code>Move _is_coercion from DefaultConvertMap to Map and make it accurate</code>
</td></tr><tr><td><a class="ext-link" href="https://git.sagemath.org/sage.git/commit/?id=7dac7e29863ee529b911213424e8a697a0b3d0ab"><span class="icon"></span>7dac7e2</a></td><td><code>Fix doctest</code>
</td></tr><tr><td><a class="ext-link" href="https://git.sagemath.org/sage.git/commit/?id=9a9e4379f68b207d1d17dcfa2ec02d220be67153"><span class="icon"></span>9a9e437</a></td><td><code>Print default "coerce" maps as "Coercion"</code>
</td></tr><tr><td><a class="ext-link" href="https://git.sagemath.org/sage.git/commit/?id=d8eec2d3e209075c7ab2a96bef095d56f2829aca"><span class="icon"></span>d8eec2d</a></td><td><code>Fixed doctests</code>
</td></tr><tr><td><a class="ext-link" href="https://git.sagemath.org/sage.git/commit/?id=2fa81a7203ac37e320acabe5b81461ccb5fab92e"><span class="icon"></span>2fa81a7</a></td><td><code>Fix doctests to print exactly as they show up on screen</code>
</td></tr><tr><td><a class="ext-link" href="https://git.sagemath.org/sage.git/commit/?id=b483c8ef214bbe14d0638406a22b67cb02afa985"><span class="icon"></span>b483c8e</a></td><td><code>Merge branch 'develop' into t/23211/mark_morphisms_as_coercions</code>
</td></tr><tr><td><a class="ext-link" href="https://git.sagemath.org/sage.git/commit/?id=78807fa8ab6560e93a85b92d6e28b8943ef2761b"><span class="icon"></span>78807fa</a></td><td><code>fix doctest errors</code>
</td></tr><tr><td><a class="ext-link" href="https://git.sagemath.org/sage.git/commit/?id=8c671637e7ba756aeff99942367b1677108c7a67"><span class="icon"></span>8c67163</a></td><td><code>Merge branch 'u/saraedum/mark_morphisms_as_coercions' of git://trac.sagemath.org/sage into public/groups/standardize_group_algebras-18700</code>
</td></tr><tr><td><a class="ext-link" href="https://git.sagemath.org/sage.git/commit/?id=a93b1e9f5b3f989c67e433e043b616073861502e"><span class="icon"></span>a93b1e9</a></td><td><code>Fixing typo and rebasing over #23211.</code>
</td></tr></table>
TickettscrimWed, 21 Jun 2017 23:36:16 GMTstatus, dependencies changed
https://trac.sagemath.org/ticket/18700#comment:76
https://trac.sagemath.org/ticket/18700#comment:76
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_review</em>
</li>
<li><strong>dependencies</strong>
changed from <em>#23000</em> to <em>#23000 #23211</em>
</li>
</ul>
<p>
I've fixed the typo, but we don't explicitly test <code>register_unpickle_override</code> anywhere else. The failure is from <a class="closed ticket" href="https://trac.sagemath.org/ticket/23211" title="enhancement: Mark morphisms as coercions (closed: fixed)">#23211</a>, but it is good to know that we have a non-trivial rebase before the next beta round.
</p>
TickettscrimFri, 23 Jun 2017 12:02:02 GMT
https://trac.sagemath.org/ticket/18700#comment:77
https://trac.sagemath.org/ticket/18700#comment:77
<p>
Perhaps we could sneak this into 8.0? This simplifies a lot of things when working with group algebras.
</p>
TicketgitSat, 24 Jun 2017 02:32:39 GMTcommit changed
https://trac.sagemath.org/ticket/18700#comment:78
https://trac.sagemath.org/ticket/18700#comment:78
<ul>
<li><strong>commit</strong>
changed from <em>a93b1e9f5b3f989c67e433e043b616073861502e</em> to <em>a18c48c7e4a8f3ee08851c6fd437f29aab364013</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1. New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="https://git.sagemath.org/sage.git/commit/?id=70c72801dbacc8d5fe586db451c0da6f565c0c2f"><span class="icon"></span>70c7280</a></td><td><code>Merge branch 'develop' into public/groups/standardize_group_algebras-18700</code>
</td></tr><tr><td><a class="ext-link" href="https://git.sagemath.org/sage.git/commit/?id=e613a51e82aed3e2d39576ea354bc9b874a55aa9"><span class="icon"></span>e613a51</a></td><td><code>Merge branch 'public/groups/standardize_group_algebras-18700' of git://trac.sagemath.org/sage into public/groups/standardize_group_algebras-18700</code>
</td></tr><tr><td><a class="ext-link" href="https://git.sagemath.org/sage.git/commit/?id=a18c48c7e4a8f3ee08851c6fd437f29aab364013"><span class="icon"></span>a18c48c</a></td><td><code>Adding test for old pickle.</code>
</td></tr></table>
TicketgitSat, 24 Jun 2017 02:33:03 GMTcommit changed
https://trac.sagemath.org/ticket/18700#comment:79
https://trac.sagemath.org/ticket/18700#comment:79
<ul>
<li><strong>commit</strong>
changed from <em>a18c48c7e4a8f3ee08851c6fd437f29aab364013</em> to <em>3160e26eb25eb09fc7164f020783da2122b18ffd</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="https://git.sagemath.org/sage.git/commit/?id=3160e26eb25eb09fc7164f020783da2122b18ffd"><span class="icon"></span>3160e26</a></td><td><code>Adding test for old pickle.</code>
</td></tr></table>
TickettscrimSat, 24 Jun 2017 02:33:56 GMT
https://trac.sagemath.org/ticket/18700#comment:80
https://trac.sagemath.org/ticket/18700#comment:80
<p>
I've added a test with an old pickle to make sure it works.
</p>
Ticketbsalisbury1Sat, 24 Jun 2017 20:36:45 GMTstatus changed
https://trac.sagemath.org/ticket/18700#comment:81
https://trac.sagemath.org/ticket/18700#comment:81
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>positive_review</em>
</li>
</ul>
TicketnthierySun, 25 Jun 2017 01:30:09 GMT
https://trac.sagemath.org/ticket/18700#comment:82
https://trac.sagemath.org/ticket/18700#comment:82
<p>
Hi!
</p>
<p>
Could you update the title and summary to briefly describe what was actually implemented in this ticket?
</p>
<p>
Thanks,
</p>
<blockquote>
<p>
Nicolas
</p>
</blockquote>
TickettscrimSun, 25 Jun 2017 02:34:26 GMTdescription, summary changed
https://trac.sagemath.org/ticket/18700#comment:83
https://trac.sagemath.org/ticket/18700#comment:83
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/18700?action=diff&version=83">diff</a>)
</li>
<li><strong>summary</strong>
changed from <em>Use GroupAlgebra as the standard class for group algebras</em> to <em>Have GroupAlgebra(Q, R) and G.algebra(R) return the same standard class for group algebras</em>
</li>
</ul>
TicketvbraunSun, 25 Jun 2017 15:45:24 GMTstatus, branch changed; resolution set
https://trac.sagemath.org/ticket/18700#comment:84
https://trac.sagemath.org/ticket/18700#comment:84
<ul>
<li><strong>status</strong>
changed from <em>positive_review</em> to <em>closed</em>
</li>
<li><strong>resolution</strong>
set to <em>fixed</em>
</li>
<li><strong>branch</strong>
changed from <em>public/groups/standardize_group_algebras-18700</em> to <em>3160e26eb25eb09fc7164f020783da2122b18ffd</em>
</li>
</ul>
Ticket