Opened 5 years ago

Closed 5 years ago

#18336 closed enhancement (fixed)

Give a default behavious to algebra_generators

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

Description (last modified by tscrim)

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

Change History (16)

comment:1 Changed 5 years ago by virmaux

  • Type changed from PLEASE CHANGE to enhancement

comment:2 Changed 5 years ago by virmaux

  • Branch set to u/virmaux/t/default_behaviour_algebra_generators

comment:3 Changed 5 years ago by virmaux

  • Commit set to 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 5 years ago by virmaux (previous) (diff)

comment:4 Changed 5 years ago by tscrim

  • 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 5 years ago by virmaux

  • Status changed from new to needs_review

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

comment:6 Changed 5 years ago by nthiery

  • Branch changed from u/virmaux/t/default_behaviour_algebra_generators to u/nthiery/t/default_behaviour_algebra_generators

comment:7 Changed 5 years ago by git

  • Commit changed from ceb8afd50e10cb316ba2db86310ce4b7390ba434 to 6a7b4f59ae87964d218bae3f75c757421eeb3f85

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

6a7b4f516659: fixed crosslinks

comment:8 Changed 5 years ago by nthiery

  • Branch changed from u/nthiery/t/default_behaviour_algebra_generators to u/virmaux/t/default_behaviour_algebra_generators
  • Commit changed from 6a7b4f59ae87964d218bae3f75c757421eeb3f85 to ceb8afd50e10cb316ba2db86310ce4b7390ba434
  • Reviewers set to Nicolas M. Thiéry
  • Status changed from needs_review to positive_review

Thanks!

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

comment:9 Changed 5 years ago by vbraun

  • Status changed from positive_review to needs_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 5 years ago by 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 5 years ago by nthiery

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 5 years ago by nthiery

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 5 years ago by vbraun

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 5 years ago by nthiery

  • Branch changed from u/virmaux/t/default_behaviour_algebra_generators to u/nthiery/t/default_behaviour_algebra_generators

comment:15 Changed 5 years ago by nthiery

  • Commit changed from ceb8afd50e10cb316ba2db86310ce4b7390ba434 to 6a7b4f59ae87964d218bae3f75c757421eeb3f85
  • Status changed from needs_work to positive_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 5 years ago by vbraun

  • Branch changed from u/nthiery/t/default_behaviour_algebra_generators to 6a7b4f59ae87964d218bae3f75c757421eeb3f85
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.