Opened 7 years ago
Last modified 12 months ago
#9280 needs_work enhancement
Implement an example of a graded algebra with basis, and improve the later
Reported by:  jhpalmieri  Owned by:  nthiery 

Priority:  minor  Milestone:  sage7.4 
Component:  categories  Keywords:  graded algebra 
Cc:  Merged in:  
Authors:  John Palmieri, Nicolas M. Thiéry  Reviewers:  Frédéric Chapoton, Travis Scrimshaw 
Report Upstream:  N/A  Work issues:  
Branch:  public/categories/graded_examples9280 (Commits)  Commit:  fb05e6c33b3202e0b0defb75007abd5bc7a6c5fd 
Dependencies:  #10193 #12453  Stopgaps: 
Description (last modified by )
The summary says it all. See also the patch on the SageCombinat patch server:
http://combinat.sagemath.org/hgwebdir.cgi/patches/file/tip/trac_9280gradedalgebrasexample.patch
Thanks to Jason Bandlow and Franco Saliola who participated to the improvement of the example. It now depends on #10193.
Attachments (4)
Change History (28)
comment:1 Changed 7 years ago by
 Status changed from new to needs_review
Changed 7 years ago by
comment:2 followup: ↓ 3 Changed 7 years ago by
comment:3 in reply to: ↑ 2 Changed 7 years ago by
Replying to nthiery:
Hi John,
For the record: we went through your patches with Franco and Jason, and discussed quite a bit around it. We will post here shortly an updated patch with some little suggestions.
Is it "shortly" yet? :)
comment:4 Changed 7 years ago by
 Description modified (diff)
 Summary changed from implement an example of a graded algebra with basis to Implement an example of a graded algebra with basis, and improve the later
comment:5 Changed 7 years ago by
 Description modified (diff)
comment:6 Changed 7 years ago by
In the sagecombinat patch, there are a few typos and some other issues:
 in sage/categories/graded_algebras_with_basis.py, the docstring for "degree" says "The degree of this element in the graded polynomial algebra." Delete "polynomial".
 in sage/categories/examples/graded_algebras_with_basis.py, the docstring for "one_basis" contains
'(0,...,0`)
, and I think this should be changed to``(0,...,0)``
.
 in sage/categories/examples/graded_algebras_with_basis.py, the docstring for the main class is now outdated: it still refers to "basis_function" and "_basis_fcn", which don't exist any more, and also to "homogeneous_component", which is now part of the default implementation, not something specific to this example.
I'm attaching a referee patch which fixes these.
There are also some doctests for "basis" in sage/categories/graded_algebras_with_basis.py which are marked as "todo: not implemented". Do we need to wait for these to be fixed, or should we consider this ready for review? It may not be ideal, but we could also change
sage: A.basis(6) # todo: not implemented (output) Family (y^{2}, x^{3}
to
sage: A.basis(6) # todo: not implemented (output) Family (y^{2}, x^{3} sage: list(A.basis(6)) [y^{2}, x^{3}]
By the way, all tests pass with this patch and with the one from #10193. So perhaps we could also delete the commentedout part at the beginning of the example, where it says
# TODO: double check that we can now discard this function
comment:7 Changed 6 years ago by
 Dependencies set to #10193
comment:8 Changed 5 years ago by
There are a couple of patches on the sagecombinat queue experimenting with moving some of the generic methods into the category GradedAlgebraWithBasis
:
 trac_9280gradedalgebrasexamplereviewfs.patch
 trac_9280gradedalgebrasexample.patch
comment:9 Changed 5 years ago by
Sorry for the long delay for the ticket but #10193 is now ready !!
comment:10 Changed 4 years ago by
 Keywords graded algebra added
comment:11 Changed 4 years ago by
Franco, Nicolas, what can we do with this ticket ? Should we use the patch from the combinat queue or the patch here ?
Changed 4 years ago by
comment:12 Changed 4 years ago by
let me take the patch of sagecombinat as a starting point.
for the bot: apply only trac_9280gradedalgebrasexamplefs.patch
comment:13 Changed 4 years ago by
I don't know why I'm listed as an author in the file "sage/categories/examples/graded_modules_with_basis.py"; I don't think I had anything to do with that.
comment:14 Changed 4 years ago by
Changed 4 years ago by
comment:15 Changed 4 years ago by
 Status changed from needs_review to needs_work
this needs to be rebased
comment:16 Changed 4 years ago by
 Dependencies changed from #10193 to #10193 #12453
 Status changed from needs_work to needs_review
Since the graded algebras with basis example is using (weighted) integer vectors, we need #12453. I'd like to attach the branch "public/categories/graded_examples9280", but trac is giving me an error when I try...
comment:17 Changed 4 years ago by
 Branch set to public/categories/graded_examples9280
 Commit set to 32dc8072c40a03e67e4dffb3075cb990b988ccb1
 Reviewers set to Frederic Chapoton, Travis Scrimshaw
New commits:
32dc807  Fixed failing doctests.

c467bdb  Merge branch 'public/refactor_integer_vectors12453' into public/categories/graded_examples9280

ef2ddce  #12453: Refactored IntegerVectors to use ClonableIntArray.

c82765f  Initial review changes.

e2c3ae8  Merge branch 'public/categories/examples9280' into public/categoires/graded_example9280

6fffda7  Merge branch 'master' into public/categories/examples9280

3491e78  imported patch trac_9280_nomodule.patch

comment:18 Changed 4 years ago by
 Reviewers changed from Frederic Chapoton, Travis Scrimshaw to Frédéric Chapoton, Travis Scrimshaw
comment:19 Changed 4 years ago by
 Commit changed from 32dc8072c40a03e67e4dffb3075cb990b988ccb1 to 2346b02544295e7df084dd65b4f509e59cfe9584
Branch pushed to git repo; I updated commit sha1. New commits:
161cedb  Merge branch 'develop' into public/categories/graded_examples9280

badae2a  Merge branch 'develop' into public/categories/graded_examples9280

eb793cc  Merge branch 'develop' into public/refactor_integer_vectors12453

b9f278e  Merge branch 'develop' into public/refactor_integer_vectors12453

2346b02  Merge branch 'public/refactor_integer_vectors12453' into public/categories/graded_examples9280

comment:20 Changed 4 years ago by
 Commit changed from 2346b02544295e7df084dd65b4f509e59cfe9584 to fb05e6c33b3202e0b0defb75007abd5bc7a6c5fd
comment:22 Changed 2 years ago by
 Milestone set to sage6.8
comment:23 Changed 12 months ago by
Yet another instance of someone asking a question related to this. Six (!) years ago, when I opened this ticket, I thought it would be good to have an example in the Sage library and in the documentation, and I really can't understand why this hasn't been taken care of yet. I am not interested in working on it myself any more, but I find it incredibly frustrating that this ticket has languished for so long.
comment:24 Changed 12 months ago by
 Milestone changed from sage6.8 to sage7.4
Hi John,
For the record: we went through your patches with Franco and Jason, and discussed quite a bit around it. We will post here shortly an updated patch with some little suggestions.