Opened 7 years ago

Closed 7 years ago

#18678 closed enhancement (fixed)

Implement convolution_product for HopfAlgebras

Reported by: alauve Owned by:
Priority: minor Milestone: sage-6.10
Component: categories Keywords: days65, convolution product
Cc: zabrocki, saliola, sage-combinat, amypang, nthiery, virmaux, jhpalmieri, nborie, mshimo, tscrim, elixyre Merged in:
Authors: Aaron Lauve, Jean-Baptiste Priez, Amy Pang, Travis Scrimshaw Reviewers: Franco Saliola, Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: 47d3a1b (Commits, GitHub, GitLab) Commit: 47d3a1bfc17b32ae13a45d8f053858d02c7eb5bf
Dependencies: #17096 Stopgaps:

Status badges

Description (last modified by alauve)

In the study of Hopf algebras (both graded and finite dimensional), one is led to look at convolution powers of the identity, and convolution products more generally. These should be implemented in bialgebras.py.

Of possible future utility is the iterated coproduct, so I define that as well.

Change History (54)

comment:1 Changed 7 years ago by alauve

  • Branch set to u/alauve/implement_convolution_product_for_hopfalgebras

comment:2 Changed 7 years ago by saliola

  • Commit set to 58f409befffcd138a1c29caefe671a309be82408

I have some ideas for you...


New commits:

58f409bAdded convolution_product and hopf_power method

comment:3 Changed 7 years ago by saliola

  • Status changed from new to needs_review

comment:4 Changed 7 years ago by saliola

  • Status changed from needs_review to needs_work

comment:5 Changed 7 years ago by saliola

Here are some comments summarizing the live review we did at Sage Days 65:

  • The first argument arg0 in def method_name(arg0, ...): should be self; this is a Python coding convention (and not strictly necessary for the code to work, but Sage coding conventions requires this).
  • Your documentation strings need to be formatted according to Sage coding conventions. Here is a link to the relevant page in the Sage developer's guide:The docstring of a function.
  • You also need to add doctests (see the above link for details). Make sure that all parts of your function are tested (i.e., if your code handles various cases, then your doctests should test the various possible cases).
  • The function n_fold_coproduct defined within convolution_power is interesting and should be a method on its own.

comment:6 follow-up: Changed 7 years ago by jhpalmieri

Is there overlap between this ticket and #18350?

comment:7 in reply to: ↑ 6 Changed 7 years ago by saliola

Replying to jhpalmieri:

Is there overlap between this ticket and #18350?

Thanks for pointing this out! Yes, there is overlap (but not a duplicate). It will make the work on this ticket even easier.

comment:8 Changed 7 years ago by amypang

  • Cc amypang added

comment:9 Changed 7 years ago by git

  • Commit changed from 58f409befffcd138a1c29caefe671a309be82408 to 66c6c0db714ae98ccb91c369964fe0dfe1844469

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

66c6c0dmerging ticket 18350

comment:10 Changed 7 years ago by git

  • Commit changed from 66c6c0db714ae98ccb91c369964fe0dfe1844469 to a80d67fec7ca9965cc8963c2fb2000a97a569cf5

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

aeefa26new operator
a685ad1new operator: iterate coproduct and product k times
fc8726aticket 18350: revision following comments
6913b18merging ticket 18350
a80d67fadded convolution product based on code from Amy Pang

comment:11 follow-up: Changed 7 years ago by alauve

  • Authors changed from alauve to Aaron Lauve
  • Cc nthiery virmaux jhpalmieri nborie mshimo tscrim elixyre added
  • Description modified (diff)

All,

I have merged #18350 with this ticket. Some comments/questions for people in-the-know to weigh-in on...

  • I have moved .adams_operator() to bialgebras.py, and overwrote it using code from Amy Pang (amypang). Then in hopf_algebras.py I overwrote the bialgebras version, allowing for negative integer powers. (E.g., the (-2)nd convolution power of the identity is none other than the 2nd power of the antipode.)
  • In fact, while adams operators naturally belong in bialgebras.py, the present code actually belongs in bialgebras_with_basis.py---as it uses .module_morphism() and .apply_multilinear_morphism()---but this would require more rewriting than I feel qualified to handle.
  • Easy fix? When poking around for an algebra without basis---on which to test a preliminary version of my code---I noticed that Sage doesn't know that QQ[x] is a module over QQ (and hence, one cannot build QQ[x].tensor(QQ[x])`. Crazy.
  • Easy fix? Similarly, even though B = FreeAlgebra(QQ,a,b) is robust enough that B.tensor(B) doesn't throw errors, quotients are out-of-bounds again. Putting C = B.quotient_ring((a*b-b^2,)), I get an AttributeError? when asking for C.tensor(C).
  • I would like to add some morphism functionality at the parent level before setting ticket to "needs_review": given linear morphisms R,S,T for a bialgebra B, create their convolution product, a new morphism, via RST = B.convolution_product(R,S,T). However, it seems ticket #15832 will have a lot of overlap with such code, so I'll hold off on implementing it unless somebody suggests otherwise.
  • If anybody can explain why iterated coproduct followed by iterated product is slower than Amy Pang's code, I'd love to hear it.

A 'thanks' in advance for any comments.

-- Aaron

comment:12 Changed 7 years ago by git

  • Commit changed from a80d67fec7ca9965cc8963c2fb2000a97a569cf5 to e07d789ade59bfc8f0475a9ed9bb9216aac0a5ca

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

e07d789fixed doctests for added methods in hopf_algebras_with_basis.py and bialgebras.py

comment:13 in reply to: ↑ 11 ; follow-up: Changed 7 years ago by tscrim

  • Dependencies set to #18350

Replying to alauve:

I have merged #18350 with this ticket.

I set this as a dependency.

  • I have moved .adams_operator() to bialgebras.py, and overwrote it using code from Amy Pang (amypang). Then in hopf_algebras.py I overwrote the bialgebras version, allowing for negative integer powers. (E.g., the (-2)nd convolution power of the identity is none other than the 2nd power of the antipode.)

I would make these changes on #18350 directly (I'd recommend git cherry-pick the commit with that change in #18350 if it's in an isolated commit, otherwise you'll have to deal with the merge conflict).

  • In fact, while adams operators naturally belong in bialgebras.py, the present code actually belongs in bialgebras_with_basis.py---as it uses .module_morphism() and .apply_multilinear_morphism()---but this would require more rewriting than I feel qualified to handle.

Then I would move the method into BialgebrasWithBasis and put an @abstract_method(optional=True) on an empty adams_operators in Bialgebras (on #18350).

  • Easy fix? When poking around for an algebra without basis---on which to test a preliminary version of my code---I noticed that Sage doesn't know that QQ[x] is a module over QQ (and hence, one cannot build QQ[x].tensor(QQ[x])`. Crazy.

Sage is full of fun oddities like that. Personally I'm not opposed to changing the category to Algebras(QQ).WithBasis(), but even with that, I'd think there's still more work to do to get QQ[x].tensor(QQ[x]) to work...

  • Easy fix? Similarly, even though B = FreeAlgebra(QQ,a,b) is robust enough that B.tensor(B) doesn't throw errors, quotients are out-of-bounds again. Putting C = B.quotient_ring((a*b-b^2,)), I get an AttributeError? when asking for C.tensor(C).

What is a and b? I think there is an abuse going on, but as of right now I cannot even construct B.

  • I would like to add some morphism functionality at the parent level before setting ticket to "needs_review": given linear morphisms R,S,T for a bialgebra B, create their convolution product, a new morphism, via RST = B.convolution_product(R,S,T). However, it seems ticket #15832 will have a lot of overlap with such code, so I'll hold off on implementing it unless somebody suggests otherwise.

I would work on #15832 and either create a follow-up with the additional code or just add that code directly to #15832.

  • If anybody can explain why iterated coproduct followed by iterated product is slower than Amy Pang's code, I'd love to hear it.

I would add an AUTHORS: block and give Amy the credit for her code. I have no idea at this point why this is. Try running %prun the_command to get timing information and look through that data for hints. Also what exactly are you testing this with? What is your timings (in particular, the approximate slowdown factor)?

comment:14 Changed 7 years ago by git

  • Commit changed from e07d789ade59bfc8f0475a9ed9bb9216aac0a5ca to 67c0e22ea91602eea9d387fe82c02f867b381aa9

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

67c0e22added convolution_product() at parent level, added a few more tests and examples.

comment:15 Changed 7 years ago by git

  • Commit changed from 67c0e22ea91602eea9d387fe82c02f867b381aa9 to de4fa8761667d770a5153e272c0522c8367188ca

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

de4fa87In ParentMethods of bialgebras.py, added ._convolution_product_from_element() for testing purposes.

comment:16 in reply to: ↑ 13 ; follow-up: Changed 7 years ago by alauve

  • Status changed from needs_work to needs_review

Hi Travis, Comments below...

Replying to tscrim:

Replying to alauve:

I have merged #18350 with this ticket.

I set this as a dependency.

I'm sure that's the wrong thing to do, but perhaps it's also the best thing to do, given the steps I already took (merging). In fact, there is no dependency, and the first thing I did was undo all the changes JBP made to hopf_algebras.py. I think not both tickets should be completed. I'm happy to move all my edits to #18350 and delete the present ticket if that makes more sense... though I don't know how to do that either.

Alternative?: set #18350 as positive review, as is, then start looking at this one? .... I'm really not sure what is the best; see next comment for more.

  • I have moved .adams_operator() to bialgebras.py, and overwrote it using code from Amy Pang (amypang). Then in hopf_algebras.py I overwrote the bialgebras version, allowing for negative integer powers. (E.g., the (-2)nd convolution power of the identity is none other than the 2nd power of the antipode.)

I would make these changes on #18350 directly (I'd recommend git cherry-pick the commit with that change in #18350 if it's in an isolated commit, otherwise you'll have to deal with the merge conflict).

It was not an isolated commit (I've learned my lesson, thanks!). Alternative idea, implemented in present ticket: I have deleted .adams_operator() from hopf_algebras.py and from hopf_algebras_with_basis.py and instead added an attribute check, hasattr(self,'antipode') within bialgebras.py to allow for negative convolution powers of the identity.

  • In fact, while adams operators naturally belong in bialgebras.py, the present code actually belongs in bialgebras_with_basis.py---as it uses .module_morphism() and .apply_multilinear_morphism()---but this would require more rewriting than I feel qualified to handle.

Then I would move the method into BialgebrasWithBasis and put an @abstract_method(optional=True) on an empty adams_operators in Bialgebras (on #18350).

I don't know how to do the former (the move), as bialgebras_with_basis.py just contains a def not a class. Maybe there is a monkey-patching solution, overwriting whatever ParentMethods? and ElementMethods? classes that get created by the def, but I'm comfortable trying neither this solution nor overhauling the whole file (i.e., to make it look more like hopf_algebras_with_basis.py).

Is there a better solution than the latter (abstract_method)? It's not really optional... more like not presently code-able. Perhaps a better solution is to keep things in bialgebras.py, perform an attribute check, self in ModulesWithBasis and add "..TODO:: remove dependency on bases" to the docstring? Anyhow, this is what I'm doing in latest commit.

  • Easy fix? When poking around for an algebra without basis---on which to test a preliminary version of my code---I noticed that Sage doesn't know that QQ[x] is a module over QQ (and hence, one cannot build QQ[x].tensor(QQ[x])`. Crazy.

Sage is full of fun oddities like that. Personally I'm not opposed to changing the category to Algebras(QQ).WithBasis(), but even with that, I'd think there's still more work to do to get QQ[x].tensor(QQ[x]) to work...

I'm going to fiddle with this. If that's all it takes, I'll start a new ticket and it should be completed easily enough.

  • Easy fix? Similarly, even though B = FreeAlgebra(QQ,a,b) is robust enough that B.tensor(B) doesn't throw errors, quotients are out-of-bounds again. Putting C = B.quotient_ring((a*b-b^2,)), I get an AttributeError? when asking for C.tensor(C).

What is a and b? I think there is an abuse going on, but as of right now I cannot even construct B.

Sorry. the code I ran was:

sage: B = FreeAlgebra(QQ,['a','b'])
sage: a,b = B.gens()
sage: B.tensor(B)
sage: C = B.quotient_ring((a*b-b^2,))
sage: C.tensor(C)
  • I would like to add some morphism functionality at the parent level before setting ticket to "needs_review": given linear morphisms R,S,T for a bialgebra B, create their convolution product, a new morphism, via RST = B.convolution_product(R,S,T). However, it seems ticket #15832 will have a lot of overlap with such code, so I'll hold off on implementing it unless somebody suggests otherwise.

I would work on #15832 and either create a follow-up with the additional code or just add that code directly to #15832.

Found an alternative solution in present commit. If ever #15832 gets adopted, the code here can be shortened by a few lines at most. Note 1: But see comments in sage.categories.bialgebras.ParentMethods.convolution_product regarding speed. Note 2: Also, I do not know how to keep .convolution_product() from showing up in the list of methods available to, e.g., sym=SymmetricFunctions(QQ). Any ideas on how to remove this? (At present, the code assumes self is something more like SymmetricFunctions(QQ).m() If the check wasn't there, Sage would throw an error, because sym.tensor(sym) doesn't work.)...

sage: sym = SymmetricFunctions(QQ); m = sym.m()
sage: Id3 = m.convolution_product((lambda x: x,)*3)
sage: Id3(m[1]) == 3*m[1]
True
sage: sym.con <TAB>
sym.construction         sym.convert_map_from     sym.convolution_product
sage: sym.convolution_product((lambda x: x,)*3)
AttributeError: `self` (Symmetric Functions over Rational Field) must belong to ModulesWithBasis. Try defining convolution product on a basis of `self` instead.
  • If anybody can explain why iterated coproduct followed by iterated product is slower than Amy Pang's code, I'd love to hear it.

I would add an AUTHORS: block and give Amy the credit for her code. I have no idea at this point why this is. Try running %prun the_command to get timing information and look through that data for hints. Also what exactly are you testing this with? What is your timings (in particular, the approximate slowdown factor)?

I have included a second ParentMethods? command, _convolution_product_from_elements() for others to investigate. I have looked at %prun using SymmetricFunctions(QQ).m() and SymmetricGroupAlgebra(QQ,6), which have radically different Hopf structure (and number of terms in product/coproduct). In both cases, _convolution_product_from_elements() (based on Amy's code) destroys convolution_product() (based on my code and Jean-Baptiste's code). I can't glean much, other than that total number of operations are vastly different.

sage: Id = lambda x: x
sage: m = SymmetricFunctions(QQ).m(); 
sage: mx = m[5,4,3,2,1]
sage: T1 = m.convolution_product((Id,)*4)
sage: timeit('T1(mx)',number=2,repeat=1)
2 loops, best of 1: 32 s per loop
sage: T2 = m._convolution_product_from_elements((Id,)*4)
sage: timeit('T2(mx)',number=2,repeat=1)
2 loops, best of 1: 32.3 s per loop

sage: mx = m[5,4,3,2,1]
sage: T1 = m.convolution_product((Id,)*7)
sage: timeit('T1(mx)',number=2,repeat=1)
2 loops, best of 1: 169 s per loop
sage: T2 = m._convolution_product_from_elements((Id,)*7)
sage: timeit('T2(mx)',number=2,repeat=1)
2 loops, best of 1: 80.3 s per loop

sage: qs = SymmetricGroupAlgebra(QQ,7); qsx = qs.an_element()^2; qsx
14*[1, 2, 3, 4, 5, 6, 7] + 4*[1, 2, 3, 4, 5, 7, 6] + 6*[1, 2, 3, 4, 6, 5, 7] + 6*[1, 2, 3, 4, 6, 7, 5] + 6*[1, 2, 3, 4, 7, 5, 6] + 2*[6, 1, 2, 3, 4, 5, 7] + [6, 7, 1, 2, 3, 4, 5] + 2*[7, 1, 2, 3, 4, 5, 6] + 5*[7, 1, 2, 3, 4, 6, 5] + 3*[7, 1, 2, 3, 5, 4, 6]
sage: T1 = qs.convolution_product(Id,Id,Id)
sage: timeit('T1(qsx)')
125 loops, best of 3: 3.98 ms per loop
sage: T2 = qs._convolution_product_from_elements(Id,Id,Id)
sage: timeit('T2(qsx)')
125 loops, best of 3: 3.4 ms per loop

sage: T1 = qs.convolution_product((Id,)*7)
sage: timeit('T1(qsx)')
25 loops, best of 3: 10.6 ms per loop
sage: T2 = qs._convolution_product_from_elements((Id,)*7)
sage: timeit('T2(qsx)')
25 loops, best of 3: 8.25 ms per loop

New commits:

de4fa87In ParentMethods of bialgebras.py, added ._convolution_product_from_element() for testing purposes.

comment:17 in reply to: ↑ 16 Changed 7 years ago by tscrim

Replying to alauve:

Replying to tscrim:

Replying to alauve:

I have merged #18350 with this ticket.

I set this as a dependency.

I'm sure that's the wrong thing to do, but perhaps it's also the best thing to do, given the steps I already took (merging). In fact, there is no dependency, and the first thing I did was undo all the changes JBP made to hopf_algebras.py. I think not both tickets should be completed. I'm happy to move all my edits to #18350 and delete the present ticket if that makes more sense... though I don't know how to do that either.

Alternative?: set #18350 as positive review, as is, then start looking at this one? .... I'm really not sure what is the best; see next comment for more.

If they are separate tickets (with separate functionality), then you should not have merged #18350 in with this ticket, and we'll have to untangle them. Probably start with a clean branch and copy your current state into the new branch.

Otherwise one has to be a dependency of the other (although not necessarily in the current order), which from my understanding should be the case because there is overlap.

  • I have moved .adams_operator() to bialgebras.py, and overwrote it using code from Amy Pang (amypang). Then in hopf_algebras.py I overwrote the bialgebras version, allowing for negative integer powers. (E.g., the (-2)nd convolution power of the identity is none other than the 2nd power of the antipode.)

I would make these changes on #18350 directly (I'd recommend git cherry-pick the commit with that change in #18350 if it's in an isolated commit, otherwise you'll have to deal with the merge conflict).

It was not an isolated commit (I've learned my lesson, thanks!). Alternative idea, implemented in present ticket: I have deleted .adams_operator() from hopf_algebras.py and from hopf_algebras_with_basis.py and instead added an attribute check, hasattr(self,'antipode') within bialgebras.py to allow for negative convolution powers of the identity.

I'm not very good about isolating commits either (I try, but often forget and typically it's not a problem). I would instead either let the method fail (this is what I would do), use a try-catch for the attribute error (this is more pythonic than using hasattr), or check if the input is in the category of Hopf algebras.

  • In fact, while adams operators naturally belong in bialgebras.py, the present code actually belongs in bialgebras_with_basis.py---as it uses .module_morphism() and .apply_multilinear_morphism()---but this would require more rewriting than I feel qualified to handle.

Then I would move the method into BialgebrasWithBasis and put an @abstract_method(optional=True) on an empty adams_operators in Bialgebras (on #18350).

I don't know how to do the former (the move), as bialgebras_with_basis.py just contains a def not a class. Maybe there is a monkey-patching solution, overwriting whatever ParentMethods? and ElementMethods? classes that get created by the def, but I'm comfortable trying neither this solution nor overhauling the whole file (i.e., to make it look more like hopf_algebras_with_basis.py).

I can set this up for you.

Is there a better solution than the latter (abstract_method)? It's not really optional... more like not presently code-able. Perhaps a better solution is to keep things in bialgebras.py, perform an attribute check, self in ModulesWithBasis and add "..TODO:: remove dependency on bases" to the docstring? Anyhow, this is what I'm doing in latest commit.

If you feel better about it, then just leave it out altogether if you can't use the Bialgebras generic code. Although you can add a .. TODO:: in the WithBasis implementation.

  • Easy fix? When poking around for an algebra without basis---on which to test a preliminary version of my code---I noticed that Sage doesn't know that QQ[x] is a module over QQ (and hence, one cannot build QQ[x].tensor(QQ[x])`. Crazy.

Sage is full of fun oddities like that. Personally I'm not opposed to changing the category to Algebras(QQ).WithBasis(), but even with that, I'd think there's still more work to do to get QQ[x].tensor(QQ[x]) to work...

I'm going to fiddle with this. If that's all it takes, I'll start a new ticket and it should be completed easily enough.

I think it deserves a ticket at the very least to keep a record of what we want Sage to do.

  • Easy fix? Similarly, even though B = FreeAlgebra(QQ,a,b) is robust enough that B.tensor(B) doesn't throw errors, quotients are out-of-bounds again. Putting C = B.quotient_ring((a*b-b^2,)), I get an AttributeError? when asking for C.tensor(C).

What is a and b? I think there is an abuse going on, but as of right now I cannot even construct B.

Sorry. the code I ran was:

sage: B = FreeAlgebra(QQ,['a','b'])
sage: a,b = B.gens()
sage: B.tensor(B)
sage: C = B.quotient_ring((a*b-b^2,))
sage: C.tensor(C)

I don't think we have the framework to currently do this, nor do I know how much it would take to do this. Nicolas, do you know off-hand?

Note 2: Also, I do not know how to keep .convolution_product() from showing up in the list of methods available to, e.g., sym=SymmetricFunctions(QQ). Any ideas on how to remove this? (At present, the code assumes self is something more like SymmetricFunctions(QQ).m() If the check wasn't there, Sage would throw an error, because sym.tensor(sym) doesn't work.)...

sage: sym = SymmetricFunctions(QQ); m = sym.m()
sage: Id3 = m.convolution_product((lambda x: x,)*3)
sage: Id3(m[1]) == 3*m[1]
True
sage: sym.con <TAB>
sym.construction         sym.convert_map_from     sym.convolution_product
sage: sym.convolution_product((lambda x: x,)*3)
AttributeError: `self` (Symmetric Functions over Rational Field) must belong to ModulesWithBasis. Try defining convolution product on a basis of `self` instead.

I would say this is a shortcoming with the WithRealizations framework and we would need to add some more magic to the category code's black magic. While I know some of it's dark and mysterious powers, I don't know if I could make this change. Nicolas, thoughts?

  • If anybody can explain why iterated coproduct followed by iterated product is slower than Amy Pang's code, I'd love to hear it.

I would add an AUTHORS: block and give Amy the credit for her code. I have no idea at this point why this is. Try running %prun the_command to get timing information and look through that data for hints. Also what exactly are you testing this with? What is your timings (in particular, the approximate slowdown factor)?

I have included a second ParentMethods? command, _convolution_product_from_elements() for others to investigate. I have looked at %prun using SymmetricFunctions(QQ).m() and SymmetricGroupAlgebra(QQ,6), which have radically different Hopf structure (and number of terms in product/coproduct). In both cases, _convolution_product_from_elements() (based on Amy's code) destroys convolution_product() (based on my code and Jean-Baptiste's code). I can't glean much, other than that total number of operations are vastly different.

I only see a big gain in the one test, but I'll take a look at it.

comment:18 Changed 7 years ago by tscrim

So the setup for the bialgebras is on the branch u/tscrim/category_hopf_algebras_with_basis, which is based on 6.8.beta7. Thus you can just pull/merge it into your branch and go from there.

For the speed difference, I believe it is due to the overhead of the intermediary objects. For example, calling tensor and the elements that are created in between the application of the morphisms. However I haven't looked too deeply into it.

comment:19 Changed 7 years ago by saliola

  • Branch changed from u/alauve/implement_convolution_product_for_hopfalgebras to u/saliola/implement_convolution_product_for_hopfalgebras

comment:20 Changed 7 years ago by git

  • Commit changed from de4fa8761667d770a5153e272c0522c8367188ca to 1d5853b2c72f5685a8543a21ef6b89dce7f13abb

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

1d5853b18678: fix some doctests

comment:21 Changed 7 years ago by saliola

I made a few improvements (I hope you think they are) to the documentation and examples.

I also added some "TODO" items in the code (numbers at the begin of the line are the line numbers):

132:            TODO: finish the above sentence ::

163:            # TODO : test this if statement and explain what it means to be flexible

324:            TODO : what is the next line testing? ::

333:            TODO : what is the next line testing? ::

408:            # TODO : test this if statement and explain what it means to be flexible

A question:

  • Does this depend on #18350? or is this intended to subsume it?

comment:22 Changed 7 years ago by git

  • Commit changed from 1d5853b2c72f5685a8543a21ef6b89dce7f13abb to 2e9f13338d90140da9a5595766309bcd887abb35

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

2e9f13318678: deal with some todos; add others

comment:23 Changed 7 years ago by saliola

  • Dependencies #18350 deleted

#18350 should be marked as a duplicate of this ticket

comment:24 Changed 7 years ago by git

  • Commit changed from 2e9f13338d90140da9a5595766309bcd887abb35 to af81d22153715768a2b0dad6415e09e54108c2c1

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

30f68ceCreated a category for BialgebrasWithBasis.
af81d22Merge branch 'u/tscrim/category_hopf_algebras_with_basis' into t/18678/implement_convolution_product_for_hopfalgebras

comment:25 Changed 7 years ago by saliola

  • Authors changed from Aaron Lauve to Aaron Lauve, Jean-Baptiste Priez, Amy Pang, Travis Scrimshaw
  • Reviewers set to Franco Saliola
  • Status changed from needs_review to needs_work
  • Work issues set to move code from bialgebras.py to bialgberas_with_basis.py; build and check documentation

I've asked that #18350 be marked as a duplicate of this ticket.

I've also added Jean-Baptiste, Amy and Travis to the authors field since this branch includes contributions from them as well.

comment:26 Changed 7 years ago by git

  • Commit changed from af81d22153715768a2b0dad6415e09e54108c2c1 to 683fe25c3d5338c339da0630bc7ab9492dbb6f10

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

683fe25Merge branch 'develop' into t/18678/implement_convolution_product_for_hopfalgebras

comment:27 Changed 7 years ago by alauve

  • Branch changed from u/saliola/implement_convolution_product_for_hopfalgebras to u/alauve/implement_convolution_product_for_hopfalgebras

comment:28 Changed 7 years ago by git

  • Commit changed from 683fe25c3d5338c339da0630bc7ab9492dbb6f10 to 1e6422ee9448099d348c3785dac9a936b925fb56

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

cd0fc4718678: improve documentation and add TODOs
1d5853b18678: fix some doctests
2e9f13318678: deal with some todos; add others
30f68ceCreated a category for BialgebrasWithBasis.
af81d22Merge branch 'u/tscrim/category_hopf_algebras_with_basis' into t/18678/implement_convolution_product_for_hopfalgebras
683fe25Merge branch 'develop' into t/18678/implement_convolution_product_for_hopfalgebras
1e6422efixed merge conflicts

comment:29 Changed 7 years ago by git

  • Commit changed from 1e6422ee9448099d348c3785dac9a936b925fb56 to d01c4dacf3896a967f82688a47e9d7b5cf30d8f3

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

d01c4damoved convolution code from bialgebras to bialgebras_with_basis. replaced the slow code in ParentMethods (based on composition of module morphisms) with faster code based on Amy Pang's algorithm.

comment:30 Changed 7 years ago by jhpalmieri

Two quick comments: first, on line 119 of bialgebras_with_basis.py, there should be a double colon at the end of the line:

identity map on group algebra of the symmetric group::

Second, the new docstrings should start with r""", not just """, at least if they contain any backslashes as parts of TeX commands.

comment:31 Changed 7 years ago by git

  • Commit changed from d01c4dacf3896a967f82688a47e9d7b5cf30d8f3 to 3664687523bd62a0d9f35e317407dc1186f02092

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

36646871. Made the changes suggested by jhpalmieri:

comment:32 Changed 7 years ago by alauve

  1. Made the changes suggested by jhpalmieri.
  1. Added some missing import calls causing convolution_product() and coproduct_iterated() methods to fail.

comment:33 Changed 7 years ago by alauve

  • Milestone changed from sage-6.8 to sage-6.9

comment:34 Changed 7 years ago by alauve

  • Status changed from needs_work to needs_review

comment:35 Changed 7 years ago by git

  • Commit changed from 3664687523bd62a0d9f35e317407dc1186f02092 to bf2279ed831a90850e5cc3777dc69ce7ca67f555

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

bf2279eMerge branch 'develop' into t/18678/implement_convolution_product_for_hopfalgebras

comment:36 Changed 7 years ago by alauve

  • Work issues move code from bialgebras.py to bialgberas_with_basis.py; build and check documentation deleted

comment:37 Changed 7 years ago by tscrim

  • Branch changed from u/alauve/implement_convolution_product_for_hopfalgebras to public/algebras/convolution_product_hopf_algebras-18678
  • Commit changed from bf2279ed831a90850e5cc3777dc69ce7ca67f555 to 50ecfe4cbfb07f3c681979860f2d69b8508ffd25
  • Reviewers changed from Franco Saliola to Franco Saliola, Travis Scrimshaw

I've made some changes:

  • I reworked the documentation around a bit.
  • I moved coproduct_iterated to CoalgebrasWithBasis since it is well-defined there.
  • I updated a few doctests for trivial failures (because of the added explicit category for BialgebrasWithBasis).

I'm thinking this is in good enough shape to merge into Sage as-is, and if there are any issues/improvements, then we can handle those on followup tickets. Thoughts?


For future reference, we will probably have (and want) to implement a custom subclass of Morphism for convolution products of morphisms for bialgebras without a specified basis (but considering we don't have any in Sage currently, I don't think this is a high priority).


New commits:

a1d962apyflakes cleanup of bialgebras.py.
50ecfe4Reviewer changes for #18678.

comment:38 Changed 7 years ago by alauve

Quoting an earlier comment:


Aaron said: I do not know how to keep .convolution_product() from showing up in the list of methods available to, e.g., sym=SymmetricFunctions(QQ). Any ideas on how to remove this?

Travis said: I would say this is a shortcoming with the WithRealizations framework and we would need to add some more magic to the category code's black magic. While I know some of it's dark and mysterious powers, I don't know if I could make this change. Nicolas, thoughts?


Did you perform the black magic? It seems .convolution_product() is no longer available to, e.g., SymmetricFunctions(QQ).

Last edited 7 years ago by alauve (previous) (diff)

comment:39 Changed 7 years ago by tscrim

It's a tome hidden deep within the forest by the category wizard(s). Actually, it's more of we don't have a mechanism for special casing the WithRealizations categories, and I might be able to actually do it. I'd open up another ticket for that.

The reason why it vanished from Sym = SymmetricFunctions(QQ) is because Sym is not a coalgebra with a (distinguished) basis.

comment:40 Changed 7 years ago by zabrocki

  • Cc zabrocki added

comment:41 Changed 7 years ago by alauve

Thanks for all the work, Travis. I'm happy with it, as-is.

Shall I set it to "positive review"?

comment:42 Changed 7 years ago by tscrim

Yep (and if anyone has any objections, they can set it to needs_work). Thank you all for your work on this.

Last edited 7 years ago by tscrim (previous) (diff)

comment:43 Changed 7 years ago by alauve

  • Status changed from needs_review to positive_review

comment:44 Changed 7 years ago by vbraun

  • Status changed from positive_review to needs_work

Doctests fail

comment:45 Changed 7 years ago by tscrim

  • Status changed from needs_work to positive_review

I don't get any doctest failures with the current branch on beta7. Could you post more information or tell us some potential conflicts from things you've already merged into your working branch if you're still getting failures?

comment:46 Changed 7 years ago by vbraun

  • Status changed from positive_review to needs_work

Still fails, duh. Try again with beta8.

comment:47 Changed 7 years ago by tscrim

Thank you for your very useful information.

comment:48 Changed 7 years ago by git

  • Commit changed from 50ecfe4cbfb07f3c681979860f2d69b8508ffd25 to ca0f846ca4ab829c3356271233d3bce3e9a31e76

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

84566ccMerge branch 'public/algebras/convolution_product_hopf_algebras-18678' of trac.sagemath.org:sage into public/algebras/convolution_product_hopf_algebras-18678
ca0f846Fixing doctest failures.

comment:49 Changed 7 years ago by tscrim

  • Status changed from needs_work to positive_review

Trivial failures due to the category hierarchy changing.

comment:50 Changed 7 years ago by vbraun

  • Status changed from positive_review to needs_work

merge conflict, wait for the next beta

comment:51 Changed 7 years ago by git

  • Commit changed from ca0f846ca4ab829c3356271233d3bce3e9a31e76 to 47d3a1bfc17b32ae13a45d8f053858d02c7eb5bf

Branch pushed to git repo; I updated commit sha1. Last 10 new commits:

570bc49Merge branch 'public/categories/super_categories-18044' into 6.9.b1
b91cd82trac #18044 correct one typo in the doc
7fd1df2Merge branch 'public/categories/super_categories-18044' of trac.sagemath.org:sage into public/categories/super_categories-18044
0579337Some reviewer tweaks and doc additions.
aec22ccAdded one more test.
4b2046fMerge branch 'public/categories/super_categories-18044' into public/categories/filtered_algebras-17096
3f67b6bFixing trivial doctest due to changes in category heirarchy.
fa476ddFixing double-colon in INPUT block.
6cc8b84Reviewer changes with Darij.
47d3a1bMerge branch 'public/categories/filtered_algebras-17096' of trac.sagemath.org:sage into public/algebras/convolution_product_hopf_algebras-18678

comment:52 Changed 7 years ago by tscrim

  • Dependencies set to #17096
  • Status changed from needs_work to positive_review

Fixed what should be the merge conflict.

comment:53 Changed 7 years ago by tscrim

  • Milestone changed from sage-6.9 to sage-6.10

comment:54 Changed 7 years ago by vbraun

  • Branch changed from public/algebras/convolution_product_hopf_algebras-18678 to 47d3a1bfc17b32ae13a45d8f053858d02c7eb5bf
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.