Opened 7 years ago

Closed 7 years ago

#18336 closed enhancement (fixed)

Give a default behavious to algebra_generators

Reported by: Aladin Virmaux Owned by:
Priority: major Milestone: sage-6.7
Component: algebra Keywords: algebras, categories
Cc: Nicolas M. Thiéry, Travis Scrimshaw Merged in:
Authors: Aladin Virmaux Reviewers: Nicolas M. Thiéry
Report Upstream: N/A Work issues:
Branch: 6a7b4f5 (Commits, GitHub, GitLab) Commit: 6a7b4f59ae87964d218bae3f75c757421eeb3f85
Dependencies: Stopgaps:

Status badges

Description (last modified by Travis Scrimshaw)

Give a default behaviour to the method algebra_generators for algebras_with_basis by calling basis().

Change History (16)

comment:1 Changed 7 years ago by Aladin Virmaux

Type: PLEASE CHANGEenhancement

comment:2 Changed 7 years ago by Aladin Virmaux

Branch: u/virmaux/t/default_behaviour_algebra_generators

comment:3 Changed 7 years ago by Aladin Virmaux

Commit: ceb8afd50e10cb316ba2db86310ce4b7390ba434

Namely we return the basis as a family of generators.

Without patch:

sage: D4 = DescentAlgebra(QQ, 4).B()
sage: D4.algebra_generators()
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
[...]
TypeError: 'NotImplementedType' object is not callable
sage: R.<x> = ZZ[]
sage: P = PartitionAlgebra(1, x, R)
sage: P.algebra_generators()
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
[..]
Error: 'NotImplementedType' object is not callable

With patch:

sage: D4 = DescentAlgebra(QQ, 4).B()
sage: D4.algebra_generators()
Lazy family (Term map from Compositions of 4 to Descent algebra of 4 over Rational Field in the subset basis(i))_{i in Compositions of 4}
sage: R.<x> = ZZ[]
sage: P = PartitionAlgebra(1, x, R)
sage: P.algebra_generators()
Finite family {{{-1, 1}}: P[{{-1, 1}}], {{-1}, {1}}: P[{{-1}, {1}}]}

It does not override specific class methods:

sage: A4 = SymmetricGroup(4).algebra(QQ)
sage: A4 in AlgebrasWithBasis(QQ)
True
sage: A4.algebra_generators()
Family ((1,2), (1,2,3,4))

New commits:

ceb8afd18336: algebra_generators in algebras_with_basis
Last edited 7 years ago by Aladin Virmaux (previous) (diff)

comment:4 Changed 7 years ago by Travis Scrimshaw

Description: modified (diff)

Is this ready for review? Also without the patch, you're able to create the descent algebra, correct (i.e., your missing the call to algebra_generators in your example)?

comment:5 Changed 7 years ago by Aladin Virmaux

Status: newneeds_review

Oops, indeed, it does work (see the exemples in the code).

comment:6 Changed 7 years ago by Nicolas M. Thiéry

Branch: u/virmaux/t/default_behaviour_algebra_generatorsu/nthiery/t/default_behaviour_algebra_generators

comment:7 Changed 7 years ago by git

Commit: ceb8afd50e10cb316ba2db86310ce4b7390ba4346a7b4f59ae87964d218bae3f75c757421eeb3f85

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

6a7b4f516659: fixed crosslinks

comment:8 Changed 7 years ago by Nicolas M. Thiéry

Branch: u/nthiery/t/default_behaviour_algebra_generatorsu/virmaux/t/default_behaviour_algebra_generators
Commit: 6a7b4f59ae87964d218bae3f75c757421eeb3f85ceb8afd50e10cb316ba2db86310ce4b7390ba434
Reviewers: Nicolas M. Thiéry
Status: needs_reviewpositive_review

Thanks!

Proofread and good to go! Aladin double checked my changes over my shoulder.

comment:9 Changed 7 years ago by Volker Braun

Status: positive_reviewneeds_work
sage -t --long src/sage/categories/algebras.py
**********************************************************************
File "src/sage/categories/algebras.py", line 160, in sage.categories.algebras.Algebras.Quotients.ParentMethods.algebra_generators
Failed example:
    S.algebra_generators()
Expected:
    Finite family {'y': B['y'], 'x': B['x'], 'b': 0, 'a': 0}
Got:
    Finite family {'y': B['y'], 'x': B['x']}
**********************************************************************
1 item had failures:
   1 of   4 in sage.categories.algebras.Algebras.Quotients.ParentMethods.algebra_generators
    [24 tests, 1 failure, 0.32 s]

comment:10 Changed 7 years ago by Aladin Virmaux

Umm is it a "bug" from Algebras.Quotients ? Here is the line to get the algebra generators of the quotient:

return self.ambient().algebra_generators().map(self.retract)

and we get generators that are 0...

But this methods should have been called instead of the newly implemented one, am I missing something ?

comment:11 Changed 7 years ago by Nicolas M. Thiéry

Algebras.WithBasis and Algebras.Quotients are independent of each other, so when you inherit from both there is an ambiguity. My guess is that Algebras.WithBasis ends up being before Algebras.Quotients in the categories of S, so the "wrong" one is picked. So we need to manually resolve this ambiguity. I'll have a look.

comment:12 Changed 7 years ago by Nicolas M. Thiéry

For info: the tests pass here; so this might depend on the context, which indeed calls for resolving the conflict.

Volker: any idea if this fails on all machines or only from time to time? Or maybe when this patch is merged together with #16659?

comment:13 Changed 7 years ago by Volker Braun

I didn't have #16659 merged when I tested this ticket, and I don't spot any obviously-conflicting ticket before that. Maybe its easiest to wait for the next beta if you can't reproduce it.

comment:14 Changed 7 years ago by Nicolas M. Thiéry

Branch: u/virmaux/t/default_behaviour_algebra_generatorsu/nthiery/t/default_behaviour_algebra_generators

comment:15 Changed 7 years ago by Nicolas M. Thiéry

Commit: ceb8afd50e10cb316ba2db86310ce4b7390ba4346a7b4f59ae87964d218bae3f75c757421eeb3f85
Status: needs_workpositive_review

Oops, I see what was the issue! There was a conflict when I concurrently pushed my branch to trac and commented on the ticket, and I accidently reset the branch to the previous one. We were lucky that the old one did not work so that this got noticed!

This is fixed now. So back to positive review. Sorry for the mess Volker!


New commits:

46f4f5418336: moved default implementation of algebra_generators to MagmaticAlgebras.WithBasis + proofread doc
6a7b4f516659: fixed crosslinks

comment:16 Changed 7 years ago by Volker Braun

Branch: u/nthiery/t/default_behaviour_algebra_generators6a7b4f59ae87964d218bae3f75c757421eeb3f85
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.