Opened 4 years ago
Closed 3 years ago
#15801 closed enhancement (fixed)
Categories over a base ring category
Reported by:  nthiery  Owned by:  

Priority:  major  Milestone:  sage6.3 
Component:  categories  Keywords:  
Cc:  sagecombinat, vbraun, SimonKing, darij  Merged in:  
Authors:  Nicolas M. Thiéry  Reviewers:  Travis Scrimshaw 
Report Upstream:  N/A  Work issues:  
Branch:  15658bd (Commits)  Commit:  15658bd13f22b874423f62b8e6e6fb4d4f7b56ec 
Dependencies:  #10963, #16275  Stopgaps: 
Description (last modified by )
Currently, to construct a category of algebras, we need to specify the base ring:
sage: Algebras(QQ)
A side effect is that the construction of e.g. GF(p)['x']
for a
lot of p
's in number theoretic calculations triggers the
construction of many parallel hierarchies of categories.
This is a waste because in most situations only the category of the base ring is relevant. In particular, the generic code provided to parents/elements/... only depends on the latter.
The purpose of this ticket is to allow for only specifying the category of the base ring, as in:
sage: Algebras(Fields())
with
sage: Algebras(QQ)
constructing a subcategory of the former. Then go trough the code,
decide in each case whether we want to use Algebras(QQ)
or
Algebras(Fields())
, and update the doctests accordingly. In most
case, the latter idiom will be preferable, unless we need some
operation on the category itself.
This would fix the regression noted on #15792 and made worse by #10963.
Further features for this ticket or followup tickets would be to:
 make
Modules(...)
into a covariant functorial constructions. This would give a proper framework for the featureModules(QQ)
>VectorSpaces(QQ)
instead of having to tackle it with a special hack.
 implement a
PolynomialRings(...)
covariant functorial construction.
Change History (73)
comment:1 Changed 4 years ago by
comment:2 Changed 4 years ago by
Replying to SimonKing:
So, the strategy would be this: The current "Category over base ring" should accept as input no a ring, but a subcategory of the category of rings.
Or more precisely, it would accept both kinds of input.
A polynomial ring should be initialised in the category of "commutative algebras over Category of quotient fields" (say), and should of course have its own reference to the base ring (say, the rational field).
Yup. And later in the category of "polynomial rings over a quotient field."
Since the parent and element classes only depend on the category of the base ring (by the current implementation), a category refinement would probably not be needed.
Yes.
But *if* needed, it would amount to take the join with the category "
FixedBaseField(QQ)
, perhaps with supercategories "FixedBaseRing(QQ)
".
Yes, though I would write it as taking the join with Algebras(QQ).
Additional technical problem: We not only have base rings, but (for bimodules) left and right base rings. Should this be reflected in the "
FixedBaseBla
" categories as well?
We will want to accept both:
Bimodules(Fields(), Fields())
and
Bimodules(QQ, QQ)
In a first step, I would not bother about mixed calls likes Bimodules(QQ, Fields)
/
And another complication: We want to have categories expressing the fact that a field (not just a ring) is acting on the objects, and then want to compute the join. Is there an easy programmatic way to let an error be raised when someone tries to compute the join "
Algebras(Fields()) & FixedBaseRing(ZZ)
"?
I don't see a trivial way to do it at this point. It certainly would be a nice feature to bark upon such constructions, but not having it is not really worst than the current situation where we have:
sage: Algebras(QQ) & Algebras(GF(3)) Join of Category of algebras over Finite Field of size 3 and Category of algebras over Rational Field
So this issue should not prevent us to proceed even if we don't find a good way out.
comment:3 followup: ↓ 4 Changed 4 years ago by
OK, that's the other possibility: Rather than having a separate mixin category FixedBaseRing(...)
, we would allow both Algebras(Fields())
and Algebras(QQ)
.
I have already done some experiments in that direction. We certainly want Algebras(R.category())
among the supercategories of Algebras(R)
. But if C
is a category, do we want that Algebras(D)
is among the supercategories of Algebras(C)
, for all D
in C.super_categories()
?
When I tried it yesterday, Nicolas' bounded C3 algorithm barked at me. Reason (if I recall correctly, I can't use my laptop right now): We want that Algebras(QQ)
and Algebras(QQ.category()
have the same parent and element classes, hence, _make_named_class_key
returns the same, and thus they also share the cmpkey used in the C3 algorithm. But the C3 algorithm expects that the items on the given lists (here: the list of all super categories) have distinct keys.
One possible approach to solve that problem: The strict supercategories of a category with fixed base ring should all be categories with unspecified base rings. Hence, Algebras(R)
should not have the supercategories Rings()
and Modules(R)
, but Algebras(R.category())
.
comment:4 in reply to: ↑ 3 ; followup: ↓ 5 Changed 4 years ago by
Replying to SimonKing:
OK, that's the other possibility: Rather than having a separate mixin category
FixedBaseRing(...)
, we would allow bothAlgebras(Fields())
andAlgebras(QQ)
.I have already done some experiments in that direction. We certainly want
Algebras(R.category())
among the supercategories ofAlgebras(R)
. But ifC
is a category, do we want thatAlgebras(D)
is among the supercategories ofAlgebras(C)
, for allD
inC.super_categories()
?
When I tried it yesterday, Nicolas' bounded C3 algorithm barked at me. Reason (if I recall correctly, I can't use my laptop right now): We want that
Algebras(QQ)
andAlgebras(QQ.category()
have the same parent and element classes, hence,_make_named_class_key
returns the same, and thus they also share the cmpkey used in the C3 algorithm. But the C3 algorithm expects that the items on the given lists (here: the list of all super categories) have distinct keys.One possible approach to solve that problem: The strict supercategories of a category with fixed base ring should all be categories with unspecified base rings. Hence,
Algebras(R)
should not have the supercategoriesRings()
andModules(R)
, butAlgebras(R.category())
.
This sounds reasonable. A variant would be, for a category over base
ring C, to set C(R).parent_class
explicitly to
C(R.category()).parent_class
, bypassing the
_make_named_class_key
business since it won't really be needed
anymore anyway (and similarly for the element class and so on).
I'd say that we certainly want Algebras(R)
to be a subcategory
of Modules(R)
, as tested by is_subcategory
. But we could
imagine doing like for joins, and *not* put Modules(R)
in
Algebras(R).<all_>super_categories()
.
In fact, maybe Algebras(R)
should really be a join category:
that of Algebras(Fields()) & Modules(R)
.
<ramble> Here Modules(R) would play a role similar to the category
with axiom C().A()
associated to the category C
defining
the axiom A
. We would not have the option to define the analogue
of a category with axioms
for a subcategory, but at this point I
don't foresee a use for it either.</ramble>
Cheers,
Nicolas
comment:5 in reply to: ↑ 4 ; followup: ↓ 7 Changed 4 years ago by
 Dependencies set to 10963
We want to build on top of #10963, right?
Replying to nthiery:
One possible approach to solve that problem: The strict supercategories of a category with fixed base ring should all be categories with unspecified base rings. Hence,
Algebras(R)
should not have the supercategoriesRings()
andModules(R)
, butAlgebras(R.category())
.This sounds reasonable. A variant would be, for a category over base ring C, to set
C(R).parent_class
explicitly toC(R.category()).parent_class
, bypassing the_make_named_class_key
business since it won't really be needed anymore anyway (and similarly for the element class and so on).
I think _make_named_class_key
would still be make sense: We need to distinguish the element and parent classes of Algebras(Fields())
from Algebras(Rings())
, but probably not from Algebras(QuotientFields())
. And in particular (when following your suggestion below) we would still want that Modules(GF(5))
and Modules(GF(7))
have the same parent and element classes.
I'd say that we certainly want
Algebras(R)
to be a subcategory ofModules(R)
, as tested byis_subcategory
. But we could imagine doing like for joins, and *not* putModules(R)
inAlgebras(R).<all_>super_categories()
.
Yes, but then one needs a category FixedBase(R)
to be joined with! And I think that's not so nice, as pointed out in comment:1. Or so I thought. But:
In fact, maybe
Algebras(R)
should really be a join category: that ofAlgebras(Fields()) & Modules(R)
.
Hmmm. Yes, in that case one wouldn't need an ugly FixedBase(R)
category...
<ramble> Here Modules(R) would play a role similar to the category with axiom
C().A()
associated to the categoryC
defining the axiomA
. We would not have the option to define the analogue of acategory with axioms
for a subcategory, but at this point I don't foresee a use for it either.</ramble>
Well, that's what I commented on at #10963: I was playing with the idea of an axiom WithFixedBase
, accepting an argument, but Volker has cut the argument short ;)
.
comment:6 Changed 4 years ago by
 Dependencies changed from 10963 to #10963
comment:7 in reply to: ↑ 5 Changed 4 years ago by
Replying to SimonKing:
We want to build on top of #10963, right?
Definitely.
Replying to nthiery: I think
_make_named_class_key
would still be make sense: We need to distinguish the element and parent classes ofAlgebras(Fields())
fromAlgebras(Rings())
, but probably not fromAlgebras(QuotientFields())
.
That would be for free if we make Algebras a functorial construction:
if a category, say QuotientFields()
, says nothing about algebras
over itself, then Algebras(QuotientFields())
will actually
return Algebras(Fields())
.
And in particular (when following your suggestion below) we would still want that
Modules(GF(5))
andModules(GF(7))
have the same parent and element classes.
Right, if we go the join way, we will need to handle the base case of Modules and have a make_named_class_key mechanism there.
comment:8 Changed 4 years ago by
Just a little note. In http://trac.sagemath.org/ticket/10963#comment:261 it was noted that base categories are referenced strongly by a global cache, and hence are prone to cause "memory leaks". This was rebutted there by noting that the number of categories created is uniformly bounded, so the memory use is manageable. This is clearly not true for these parametrized "categories with fixed base".
The work on this ticket will improve the situation, because categories parametrized by elements from an infinite set will be less often created, but it's worthwhile to try and think of what to do if they do get created. Perhaps we should really try and avoid them completely? Certainly the infrastructure requirements to support them is radically different, because such parametrized categories need to be deallocatable (I'm impressed that Firefox's spell checker accepts that word and not Firefox).
comment:9 followup: ↓ 13 Changed 4 years ago by
A simple test that detects some of the problems #10963 presently causes:
while True: k=random_matrix(Rationals(),30,20).echelon_form()
On 6.2beta4 this happily runs with constant memory footprint and about constant time per iteration.
With #10963 applied, it gradually eats memory. The reason is that "integers mod p" and their categories pile up in memory. The iterations also take about twice as long and they slow down over time (as one would expect with memory use increasing).
(see #13925 for why the choice of algorithm might not be so sensible, but it's a useful test for our purposes here)
comment:10 Changed 4 years ago by
Hi!
I started to work on this. We can now do:
sage: Algebras(Fields()) Category of algebras over Category of fields
(TODO: would be nicer as Category of algebras over fields)
and by default, polynomial rings are created in such categories:
sage: QQ['x'].category() Join of Category of euclidean domains and Category of commutative algebras over Category of quotient fields
All tests pass in the sage/rings/polynomial.
Here are the new timing for Simon's benchmark about creating lots of polynomial rings:
With Sage 5.0.1:
sage: %time test() CPU times: user 7.38 s, sys: 0.08 s, total: 7.46 s Wall time: 7.48 s
With develop (6.2.beta7):
sage: %time test() CPU times: user 9.31 s, sys: 125 ms, total: 9.44 s Wall time: 9.47 s
With #10963:
sage: %time test() CPU times: user 17.6 s, sys: 288 ms, total: 17.9 s Wall time: 17.8 s
With #10963 and this ticket:
sage: def test(): ....: for p in prime_range(10000): ....: P = GF(p)['t','x','z'] sage: %time test() CPU times: user 2.99 s, sys: 127 ms, total: 3.11 s Wall time: 3.03 s
Sounds like the timeperformance issue for polynomials is indeed fixed for polynomials :)
Now, on to matrices!
comment:11 Changed 4 years ago by
 Branch set to refs/heads/public/categories/overabaseringcategory15801
comment:12 Changed 4 years ago by
 Branch changed from refs/heads/public/categories/overabaseringcategory15801 to public/categories/overabaseringcategory15801
comment:13 in reply to: ↑ 9 Changed 4 years ago by
 Commit set to 3a63847b3e313d2e4eb0261f156e7d0bb7bf7a9c
Replying to nbruin:
A simple test that detects some of the problems #10963 presently causes:
while True: k=random_matrix(Rationals(),30,20).echelon_form()On 6.2beta4 this happily runs with constant memory footprint and about constant time per iteration.
With #10963 applied, it gradually eats memory. The reason is that "integers mod p" and their categories pile up in memory. The iterations also take about twice as long and they slow down over time (as one would expect with memory use increasing).
(see #13925 for why the choice of algorithm might not be so sensible, but it's a useful test for our purposes here)
For the record: I just checked, and with the current version of this ticket on top of #10963, this runs with constant memory footprint and about constant time per iteration.
Cheers,
Nicolas
Last 10 new commits:
1b93d6e  Merge branch 'public/ticket/10963docdistributive' of trac.sagemath.org:sage into public/ticket/10963docdistributive

28b39a2  Merge branch 'develop' into public/ticket/10963docdistributive

70972db  Merge branch 'develop' into public/ticket/10963docdistributive

e1110da  Trac 10963: added little note in Category._super_categories

ad718de  Trac 10963: added mathematical definitions to the documentation of a bunch of axioms

d24a3d5  Trac 15801: enable categories over a base ring category and use them in polynomials

6bf3893  Trac 15801: improved repr for categories over base ring categories and fixed doctest

7db4ef0  Trac 10963: Fixed repr for the symmetric group algebra by improving a bit the generic repr methods in the categories

1116ef5  Merge branch 'categories/axioms10963' into categories/over_a_base_category15801

3a63847  Trac 15801: updated doctest

comment:14 Changed 4 years ago by
Replying to nbruin:
There are some performance and leaking issues with the proposed code here. They have been mentioned before, but they got swamped in more theoretical design discussions. Consider for instance with sage 6.2.beta4:
...
As you can see, much slower execution and much leakier behaviour (even without the patch this kind of stuff is very liable to leak but as you can see, this patch makes it much worse). Note that we have to sort types by their string representation, because for each of these
..._with_category
types, the 5000 instances are in fact unique types with identical print names. So the memory load is even higher (types are fairly heavy data structures). As you can see, categories are eternal (they get cached) and since they store a reference to their base, they nail the base ring in memory too. We'll have to merge this ticket together with a ticket that moves base ring references out of the categories, since the constructions introduced in this ticket make the problem urgent.
For the record, here is what I currently get on my machine:
With this ticket applied on top of #10963:
sage: import gc sage: from collections import Counter sage: before={id(c) for c in gc.get_objects()} sage: %time for n in [1..5000]: x=(matrix(Integers(n),2,2,1)) CPU times: user 3.83 s, sys: 41.2 ms, total: 3.87 s Wall time: 3.84 s sage: sage: gc.collect() 17047 sage: sage: gc.collect() 864 sage: sage: after=[c for c in gc.get_objects() if id(c) not in before] sage: sage: [c for c in Counter([str(type(a)) for a in after]).items() if c[1] > 4000] []
Same, without systematic full category initialization for matrices:
CPU times: user 3.49 s, sys: 52.8 ms, total: 3.54 s Wall time: 3.49 s sage: sage: sage: sage: gc.collect() 18836 sage: sage: sage: sage: gc.collect() 1344
With vanilla 6.2.beta6 (no systematic full category initialization for matrices):
CPU times: user 2.89 s, sys: 33.6 ms, total: 2.92 s Wall time: 2.88 s sage: sage: sage: gc.collect() 5032 sage: sage: sage: gc.collect() 704
Hence #15801 as is seems to be going a long way toward fixing this issue. It's still non negligibly slower than develop on this very tight loop (and apparently more objects get allocated/deallocated). But in exchange we are back to systematic full category initialization which I believe is much nicer to the user.
What do you think?
On to the elliptic curve benchmarks.
Cheers,
Nicolas
comment:15 Changed 4 years ago by
On the benchmark from #11900, I get significantly the same timing with develop and #10963+#15801:
sage: E = J0(46).endomorphism_ring() sage: %time g = E.gens() CPU times: user 3.63 s, sys: 94.9 ms, total: 3.73 s Wall time: 3.90 s #develop Wall time: 3.98 s #10963 Wall time: 3.86 s #10963+#15801
This, despite the additional full category initialization.
comment:16 Changed 4 years ago by
 Commit changed from 3a63847b3e313d2e4eb0261f156e7d0bb7bf7a9c to 2121f513a9f7694c6a4c13b9803ed5df9954304b
Branch pushed to git repo; I updated commit sha1. Last 10 new commits:
f44661f  Trac 15801: added repr in HeckeModule_free_module (the generic one in modules with basis can't handle it anymore)

545765a  Merge branch 'categories/axioms10963' into categories/over_a_base_category15801

961bfb6  Trac 15801: reinstated test in vector spaces after some mess up in the #10963 branch; updated some doctests

a051864  Trac 15801: deprecates MatrixSpace.full_category_initialisation which is now unneeded

42ab3cd  Trac 15801: update thematic_tutorials/coercion_and_categories.rst w.r.t the full initialization of matrix spaces

9cc7ef4  Trac 15801: little fix in polynomial_default_category, and update doctests involving categories of polynomial rings

3d2ec28  Trac 15801: fixed recent missing import

cf847f8  Trac 15801: use a group algebra rather than a polynomial algebra in an example to keep it interesting

fa60747  Trac 15801: fixed typo in doctest

2121f51  Trac 15801: improved subcategory and containment tests for categories over a base ring category

comment:17 Changed 4 years ago by
Up to merging in #15919, it's likely that all long test will pass. Running them now.
comment:18 Changed 4 years ago by
 Commit changed from 2121f513a9f7694c6a4c13b9803ed5df9954304b to 12afbe46ebafe56cf74661bb6d5ea81af583ba83
Branch pushed to git repo; I updated commit sha1. New commits:
12afbe4  Trac 15801: fixed ReST typo

comment:19 followup: ↓ 20 Changed 4 years ago by
 Description modified (diff)
So Algebras(Rings())
would cover any algebra (well...any assoc & unital algebra)?
Also I think we should not loose support for having the category of algebras over a fixed base ring. For example, given two representations V
, W
of a group algebra RG
over R
, the morphisms from V
to W
as RG
modules is (significantly) smaller than the morphisms as R
modules. Currently our set of morphisms depends on the category, which if we keep the current setup, would give that Hom(V, W, Modules(RG)) is Hom(V, W, Modules(R))
(well, maybe only ==
).
Actually...RG
would be the category of group algebras whereas R
would be in the category of rings, unless R
was also the group algebra of some other group (okay, it's very contrived, but possible). My point is that just passing the category into Modules
doesn't always tell us the full structure (of the homset), and is there some way we can work around this?
comment:20 in reply to: ↑ 19 Changed 4 years ago by
Replying to tscrim:
So
Algebras(Rings())
would cover any algebra (well...any assoc & unital algebra)?
Yup. And at some point we will want weaker things like Algebras(SemiRings())
and so on.
Also I think we should not loose support for having the category of algebras over a fixed base ring.
I agree with that, though more if we want to manipulate the category "mathematically". From a code perspective point of view, since #11900, the provided abstract classes only depends on the category of the base ring, and this has not been a limitation so far.
For example, given two representations
V
,W
of a group algebraRG
overR
, the morphisms fromV
toW
asRG
modules is (significantly) smaller than the morphisms asR
modules. Currently our set of morphisms depends on the category, which if we keep the current setup, would give thatHom(V, W, Modules(RG)) is Hom(V, W, Modules(R))
(well, maybe only==
).Actually...
RG
would be the category of group algebras whereasR
would be in the category of rings, unlessR
was also the group algebra of some other group (okay, it's very contrived, but possible). My point is that just passing the category intoModules
doesn't always tell us the full structure (of the homset), and is there some way we can work around this?
Well, as you say, we have two distinct categories:
sage: Modules(Rings()) sage: Modules(GroupAlgebras(Rings()))
so we are fine, aren't we?
Cheers,
Nicolas
comment:21 followup: ↓ 23 Changed 4 years ago by
Until I make a group ring over a group ring. Actually, maybe a less contrived example of modules over S = R[a,b] / J
where R = ZZ[x,y] / I
for some ideals I, J
. There should be more morphisms as R
modules than as S
modules. I think this could be a general issue anytime we try to do extension of scalars (or I'm being completely paranoid).
comment:22 Changed 4 years ago by
 Commit changed from 12afbe46ebafe56cf74661bb6d5ea81af583ba83 to 0397865de0a1b9636b7501f0d666f08473b148fb
Branch pushed to git repo; I updated commit sha1. New commits:
8eaaa99  Merge branch 'develop' into public/ticket/10963docdistributive

feab04a  manual merge with 6.2.beta8

ce2193e  Merge branch 'public/ticket/10963docdistributive' of trac.sagemath.org:sage into public/ticket/10963docdistributive

268345f  Trac 10963: attempt at improving the wording for quotients/subquotients

729299b  Merge sage 6.2.beta8 together with the latest version of #10963 in #15801

0397865  Trac 15801: added warning about not yet fully implemented subcategory tests

comment:23 in reply to: ↑ 21 ; followup: ↓ 25 Changed 4 years ago by
Replying to tscrim:
Until I make a group ring over a group ring.
What about:
Modules(Groups().Algebras(Groups().Algebras()))
Actually, maybe a less contrived example of modules over
S = R[a,b] / J
whereR = ZZ[x,y] / I
for some idealsI, J
. There should be more morphisms asR
modules than asS
modules. I think this could be a general issue anytime we try to do extension of scalars (or I'm being completely paranoid).
One thing is that each module M in Sage has a distinguished "base ring".
<digression>
In particular, M can't be in Modules(R)
and in Modules(S)
simultaneously; this can be seen as a limitation of the current
category framework; no other CAS found a good solution to that
though
</digression>
So, if I have two modules M and N, Hom(M,N) actually depends on this distinguished base ring (they should have the same!).
If I build M and N as Rmodules, and M' and N' as Smodules, the fact that they all belong to the same category only means that the code to handle and compute the homsets will be the same, not that the homsets Hom(M,N) and Hom(M',N') themselves will be the same.
Cheers,
Nicolas
comment:24 Changed 4 years ago by
 Status changed from new to needs_review
comment:25 in reply to: ↑ 23 ; followup: ↓ 27 Changed 3 years ago by
Replying to nthiery:
Okay, I'm happy as long as we're getting things like this:
Modules(Groups().Algebras(Groups().Algebras()))
for things like (QQ[G])[H]
where G,H
are groups.
One thing is that each module M in Sage has a distinguished "base ring". <digression> In particular, M can't be in
Modules(R)
and inModules(S)
simultaneously; this can be seen as a limitation of the current category framework; no other CAS found a good solution to that though </digression>
<digressing as well>This might improve as (if) we implement coercions between categories.</digressing as well>
So, if I have two modules M and N, Hom(M,N) actually depends on this distinguished base ring (they should have the same!).
If I build M and N as Rmodules, and M' and N' as Smodules, the fact that they all belong to the same category only means that the code to handle and compute the homsets will be the same, not that the homsets Hom(M,N) and Hom(M',N') themselves will be the same.
Ah, I see. You must construct new modules M' and N' in order to consider morphisms as Smodules. I'm good with everything (conceptually).
Simon, Nils, (or anyone else,) did either of you verify Nicolas' above (or have any objections)?
comment:26 Changed 3 years ago by
 Commit changed from 0397865de0a1b9636b7501f0d666f08473b148fb to d396719ca5442e41259b16f43994e87099fdfcea
Branch pushed to git repo; I updated commit sha1. New commits:
18a4a74  Trac 10963: improve explanation of Sage notion of subcategories

5c271c9  Trac 10963: improve explanation of Sage notion of subcategories (proofreading/rewording)

d0664d1  Trac 10963: three typo fixes suggested by Darij

3009d90  Trac 10963: little documentation improvements suggested by Darij

02db670  Trac 10963: minor edits to "On the category hierarchy"

3bb48cb  Merge branch 'develop = 6.2.rc0' into categories/axioms10963

d396719  Merge branch '6.2.rc0' and categories/axioms10963 into categories/over_a_base_category15801

comment:27 in reply to: ↑ 25 Changed 3 years ago by
Replying to tscrim:
Replying to nthiery: Okay, I'm happy as long as we're getting things like this:
Modules(Groups().Algebras(Groups().Algebras()))for things like
(QQ[G])[H]
whereG,H
are groups.
For now the group algebra construction still uses categories over base rings. But yes, that's what would happen otherwise.
Thanks for the feedback!
comment:28 Changed 3 years ago by
 Commit changed from d396719ca5442e41259b16f43994e87099fdfcea to 6a960b2d827dc5dbd0afb81fa6c1f2e402e25db8
Branch pushed to git repo; I updated commit sha1. Last 10 new commits:
a432952  Trac 10963: trivial doctest update w.r.t. #15240 (Switch lattice polytopes to point collections)

dc87588  clarifying Subobjects and Quotients interfaces

be5150f  some questions

3a0e8f7  clarification of my clarification

86235d4  Trac 10963: clarification^3 about subobjects

08ea069  Trac 10963: suppressed now useless category example MyGroupAlgebra + tiny doc improvement

1441304  some final fixes

ab4562c  Merge branch 'public/ticket/10963docdistributive' into public/categories/overabaseringcategory15801

5b8438e  Fixing up doc and some other minor things.

6a960b2  Fixed Sets.Algebras().

comment:29 followup: ↓ 31 Changed 3 years ago by
I've merged in the latest #10963, made some misc doctweaks (added some doctests to/near functions were editing), fixed an issue with Sets.Algebras
not taking a category (so GroupAlgebras(Rings())
used to fail). If you're happy with my changes, then I think this part is a positive review.
comment:30 Changed 3 years ago by
 Commit changed from 6a960b2d827dc5dbd0afb81fa6c1f2e402e25db8 to 1f09b8bde256c00bab2359e799b88e9da2273720
Branch pushed to git repo; I updated commit sha1. New commits:
16ec0f3  some more question(able edits)

2322225  Trac 10963: Answer's to Darij's questions + minor edits + suppressed broken Monoids.Algebras.ParentMethods.an_element

c76743f  Merge branch 'categories/axioms10963' into categories/over_a_base_category15801

1f09b8b  Trac 15801: proofread Travis's comments + minor doctest improvements

comment:31 in reply to: ↑ 29 Changed 3 years ago by
Replying to tscrim:
I've merged in the latest #10963, made some misc doctweaks (added some doctests to/near functions were editing), fixed an issue with
Sets.Algebras
not taking a category (soGroupAlgebras(Rings())
used to fail). If you're happy with my changes, then I think this part is a positive review.
Cool, thanks! Please proofread my proofreading :)
I have merged the latest #10963 since I was not sure it would not conflict with my changes.
Running all doctests now with the little tweak from #15919.
Cheers,
Nicolas
comment:32 Changed 3 years ago by
If all doctests pass for you, then positive review for this.
comment:33 Changed 3 years ago by
 Status changed from needs_review to needs_work
 Work issues set to Fix failing test in modules_with_basis.py
Shoot, there is one failing doctest which I somehow missed. Some Homset is missing a base_ring method which causes it not to be detected properly as being in its category. I'll try to fix this tomorrow or so.
Cheers,
Nicolas
comment:34 Changed 3 years ago by
 Commit changed from 1f09b8bde256c00bab2359e799b88e9da2273720 to 2114d54f98b80c9542b61051600d9ff686f99f13
Branch pushed to git repo; I updated commit sha1. Last 10 new commits:
55a36d6  postapocalyptic edits (correct this time)

72663b7  Merge branch 'public/ticket/10963docdistributive' of trac.sagemath.org:sage into public/ticket/10963docdistributive

67cbc90  Merge branch 'public/ticket/10963docdistributive' into public/categories/overabaseringcategory15801

655b1f8  Trac 10963: Proofread latest changes by Darij and Travis + minor doc edits

14d4449  Trac 10963: Minor doc edit

9d7762f  Trac 10963: fixed typo

70e7b32  Fixed some more typos and back to "if not blah:".

655fc1e  Merge branch 'public/ticket/10963docdistributive' into public/categories/overabaseringcategory15801

ac91ed0  Added _Hom_() method for Modules.

2114d54  Merge branch 'public/categories/overabaseringcategory15801' of trac.sagemath.org:sage into public/categories/overabaseringcategory15801

comment:35 Changed 3 years ago by
 Reviewers set to Travis Scrimshaw
 Status changed from needs_work to needs_review
 Work issues Fix failing test in modules_with_basis.py deleted
I've fixed it by implementing a _Hom_()
for Modules
(see commit ac91ed0). This might be more of a hack solution, but it works.
comment:36 Changed 3 years ago by
Ah, yes, that could one way to handle it. Thanks!
Sorry for the silence. I have been toying with a variant of it, putting a similar test in the Homset constructor. I don't know which one is best. Yours is more local contextwise (it only affects modules); mine is more local code wise.
However while doing this I stumbled on another issue, which is that
now X in category
is not necessarily equivalent to
X.category().is_subcategory(category)
. This is ok theoretically,
but there is at least one spot in the code, namely in Hom, where this
has to be fixed, which in turn opens a can of pickling worms.
Hopefully I'll have a proposal later tonight.
Cheers,
Nicolas
comment:37 Changed 3 years ago by
 Dependencies changed from #10963 to #10963, #16275
I pushed my proposal to #16275.
Thanks to the use of X in category
for sanity checks in Hom, we
can still handle things like:
Hom(ZZ['x'], ZZ['x'], Modules(ZZ))
even if ZZx?
is now in the category of
Modules(Rings())
(which is not a subcategory of
Modules(ZZ)
).
I'll work tomorrow on double checking everything once #16275 is merged in this ticket. I might end up preferring my hack rather than the _Hom_ hack, because the whole _Hom_ protocol is not quite satisfactory, and I'd rather avoid it when we can.
What do you think?
comment:38 followup: ↓ 39 Changed 3 years ago by
I think we should leave my _Hom_
hack in because the homset is a module (when the codomain is a module as well) and at least we're somewhat demonstrating that when specifying that the homset has a base ring (through the Parent.__init__
). So I'd say my _Hom_
and #16275 should coexist in the final version.
comment:39 in reply to: ↑ 38 Changed 3 years ago by
Replying to tscrim:
I think we should leave my
_Hom_
hack in because the homset is a module (when the codomain is a module as well) and at least we're somewhat demonstrating that when specifying that the homset has a base ring (through theParent.__init__
). So I'd say my_Hom_
and #16275 should coexist in the final version.
Sorry, I was unclear. Yes, we definitely need some Hom hack to setup base_ring properly by passing it to init. My question is just whether this hack belongs to a custom _Hom_
method as you did or to Homset.__init__
as in my (yet unpushed) branch.
comment:40 Changed 3 years ago by
 Commit changed from 2114d54f98b80c9542b61051600d9ff686f99f13 to aa0159142f58b1feb9f2462aec4cc24a806165b3
Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:
1ff993b  #16275: trivial commit to use format

0074f02  #16275: added doctest illustrating an unpickling where Hom is called on an uninitialized parent

c1e91f3  #16275: update ticket number in the comments

ae3ca80  This is handled as well by the generic of Homset.__reduce__ method,

507ee00  Trac 16275: fixes for the previous commit; btw its title should have read: removed sage.modular.abvar.homspace.Homspace.__reduce__

6f55aa2  Merge branch 't/16275/hom__introduce_a_check_argument_to_simplify_the_unpickling_detection_logic' into categories/over_a_base_category15801

fe1c1c5  Trac 10963: Removed the old _join helper function in category.py (why was it still there???)

d6dc1fd  Merge branch 'categories/axioms10963' into categories/over_a_base_category15801

54c3d67  #15801: Initialize the base ring for module homsets

aa01591  #15801: doctests for CategoryObjects.base_ring + docfix in Modules.SubcategoryMethods.base_ring

comment:41 Changed 3 years ago by
I just pushed my branch. Hmm, that was a bit bruteforce, sorry: I should probably have pushed it under a different name since it does not include your change ac91ed0.
See 54c3d67 for the alternative I propose; does it sound ok?
Last 10 new commits:
1ff993b  #16275: trivial commit to use format

0074f02  #16275: added doctest illustrating an unpickling where Hom is called on an uninitialized parent

c1e91f3  #16275: update ticket number in the comments

ae3ca80  This is handled as well by the generic of Homset.__reduce__ method,

507ee00  Trac 16275: fixes for the previous commit; btw its title should have read: removed sage.modular.abvar.homspace.Homspace.__reduce__

6f55aa2  Merge branch 't/16275/hom__introduce_a_check_argument_to_simplify_the_unpickling_detection_logic' into categories/over_a_base_category15801

fe1c1c5  Trac 10963: Removed the old _join helper function in category.py (why was it still there???)

d6dc1fd  Merge branch 'categories/axioms10963' into categories/over_a_base_category15801

54c3d67  #15801: Initialize the base ring for module homsets

aa01591  #15801: doctests for CategoryObjects.base_ring + docfix in Modules.SubcategoryMethods.base_ring

comment:42 Changed 3 years ago by
 Status changed from needs_review to positive_review
Your proposal will work since any category with has a WithBasis
should have a forgetful functor to modules.
comment:43 Changed 3 years ago by
 Milestone changed from sage6.2 to sage6.3
comment:44 Changed 3 years ago by
 Commit changed from aa0159142f58b1feb9f2462aec4cc24a806165b3 to 79f476698d8658ca8649ab765de00995c24f5455
 Status changed from positive_review to needs_review
Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. Last 10 new commits:
7e07ded  Trac 10963: Fixed crosslinks

250c8fa  Merge branch 'public/ticket/10963docdistributive' of git://trac.sagemath.org/sage into 10963

16b8e0f  documentation edits to clarify that super_categories should not contain anything twice

bbfcbb8  Merge Darij's commit in 'public/ticket/10963docdistributive' into categories/axioms10963

f22508b  Trac 10963: proofread Darij's edits

8da7522  Trac 10963: degraded a bit a newly added crosslink to work around 'make docclean; make clean' failure

4ef8b27  Merge branch 'public/ticket/10963docdistributive' of git://trac.sagemath.org/sage into 10963new

afb911c  corrected conflict resolution

82c8ee0  Merge branch 'public/ticket/10963docdistributive' of trac.sagemath.org:sage into public/ticket/10963docdistributive

79f4766  Merge branch 'public/categories/overabaseringcategory15801' of trac.sagemath.org:sage into public/categories/overabaseringcategory15801

comment:45 Changed 3 years ago by
Rebased over rebased #10963.
comment:46 Changed 3 years ago by
 Status changed from needs_review to positive_review
comment:47 Changed 3 years ago by
 Status changed from positive_review to needs_work
I get lots of doctest failuers, e..g NotImplementedError: Pieri factors for type ['A', 3, 1]
. Might be this ticket or #10963. On top of 6.3.beta0.
comment:48 Changed 3 years ago by
Is this with #14102 applied? Could this be due to the import statement that was accidently removed and then reinserted in 1579b88. See http://trac.sagemath.org/ticket/14102#comment:72.
Cheers,
Nicolas
comment:49 Changed 3 years ago by
No, #14102 wasn't applied since it depends on this ticket.
comment:50 Changed 3 years ago by
 Commit changed from 79f476698d8658ca8649ab765de00995c24f5455 to 281f5392e81e98d835ddf249325e66c1c36a08cc
Branch pushed to git repo; I updated commit sha1. New commits:
281f539  Added back in import statement as per comment.

comment:51 Changed 3 years ago by
Ah, I see: the accidental removal of import
occured in this branch (commit 41768a3), and was reverted in the followup branch #14102 (commit 1579b88). I cherrypicked back 1579b88 to this branch. This should work now. Running the tests.
Thanks for your report!
Cheers,
Nicolas
comment:52 Changed 3 years ago by
 Status changed from needs_work to positive_review
comment:53 Changed 3 years ago by
 Status changed from positive_review to needs_work
 Work issues set to docpdf
make docpdf
fails (probably due to #10963)
comment:54 followup: ↓ 55 Changed 3 years ago by
Also
sage t long src/sage/quivers/path_semigroup.py # 1 doctest failed sage t long src/sage/quivers/representation.py # 6 doctests failed
comment:55 in reply to: ↑ 54 Changed 3 years ago by
comment:56 Changed 3 years ago by
 Commit changed from 281f5392e81e98d835ddf249325e66c1c36a08cc to dcb11d850530d23e1d8867daa84ade0e04d7ce15
Branch pushed to git repo; I updated commit sha1. New commits:
96c631f  Merge branch 'develop' into categories/axioms10963

b1a2aed  Trac 10963: two typo fixes to let the pdf documentation compile

c16f18b  Merge branch 'public/ticket/10963docdistributive' of trac.sagemath.org:sage into categories/axioms10963

dcb11d8  Merge branch 'categories/axioms10963' into categories/over_a_base_category15801

comment:57 Changed 3 years ago by
Pdf compilation fixed in #10963 and merged here.
comment:58 followups: ↓ 59 ↓ 61 Changed 3 years ago by
 Cc darij added
I think the quiver issue should be discussed here.
Nicolas, do you agree with Darij that quiver representations over QQ should live in Modules(QQ)
, not in Modules(QuiverAlgebra)
? If this is so, then hopefully it will be easy to switch.
comment:59 in reply to: ↑ 58 Changed 3 years ago by
Replying to SimonKing:
I think the quiver issue should be discussed here.
Ok.
Nicolas, do you agree with Darij that quiver representations over QQ should live in
Modules(QQ)
, not inModules(QuiverAlgebra)
? If this is so, then hopefully it will be easy to switch.
No strong opinion either way, but yes, I would lean in this direction
since we don't have yet a proper category hierarchy for
representations. So in the mean time, it makes sense to reserve
Modules(K)
for what we have used it so far: doing linear algebra
over K, ...
If it makes it easier to switch, so much the better :)
Cheers,
Nicolas
comment:60 Changed 3 years ago by
 Commit changed from dcb11d850530d23e1d8867daa84ade0e04d7ce15 to 15658bd13f22b874423f62b8e6e6fb4d4f7b56ec
Branch pushed to git repo; I updated commit sha1. New commits:
15658bd  Trac 15801: fixed merge with #12630

comment:61 in reply to: ↑ 58 Changed 3 years ago by
Replying to SimonKing:
Nicolas, do you agree with Darij that quiver representations over QQ should live in
Modules(QQ)
, not inModules(QuiverAlgebra)
? If this is so, then hopefully it will be easy to switch.
Done. Please review! Running all long tests now.
comment:62 Changed 3 years ago by
Sorry for my being too slow. I wanted to try and fix it, but was busy with some other tickets. Thank you for doing it, and I'll now try to review.
comment:63 Changed 3 years ago by
I read the latest commit, and it seems good. Hence, I wait for the test results...
comment:64 Changed 3 years ago by
The pdf does build...
comment:65 Changed 3 years ago by
 Work issues docpdf deleted
I get numerous errors of the following type:
File "src/sage/dev/sagedev.py", line 4836, in sage.dev.sagedev.SageDev._new_local_branch_for_ticket Failed example: dev, config, UI, server = single_user_setup() Expected nothing Got: GitError: git exited with a nonzero exit code (1). This happened while executing "git c user.email=doc@test.test c user.name=doctest pull /home/king/.sage/temp/linuxetl7.site/1419/dir_y4yz1q/repo master". git printed nothing to STDOUT. git printed the following to STDERR: git: 'pull' is not a git command. See 'git help'. <BLANKLINE> Did you mean this? shell
Never seen such errors before.
comment:66 Changed 3 years ago by
Is this perhaps an issue of sage6.3.beta0?
comment:67 followup: ↓ 72 Changed 3 years ago by
Thanks Simon for reviewing this! Could you have a quick look as well at the small last commit on #10963?
For the git error, IIRC you need a recent version of git for all tests to pass, and doing
sage i git
should do the job.
Cheers,
Nicolas
comment:68 followup: ↓ 71 Changed 3 years ago by
Simon, are you using sage git
? There is always plain git
, and I fear that it'll stay the more reliable option for a long time...
comment:69 followup: ↓ 70 Changed 3 years ago by
 Status changed from needs_work to positive_review
Tests pass for me, so I'm going to set this to positive review.
I also only use my systemwide git
.
comment:70 in reply to: ↑ 69 Changed 3 years ago by
Replying to tscrim:
I also only use my systemwide
git
.
Are you sure that you use your systemwide git
when you run the tests in sage.dev? Anyway, my systemwide git does know about git pull
only Sage's git doesn't know.
Best regards, Simon
comment:71 in reply to: ↑ 68 Changed 3 years ago by
Replying to darij:
Simon, are you using
sage git
? There is always plaingit
, and I fear that it'll stay the more reliable option for a long time...
I am now talking about the git that I use, but about the git that sage dev
uses.
comment:72 in reply to: ↑ 67 Changed 3 years ago by
Replying to nthiery:
For the git error, IIRC you need a recent version of git for all tests to pass, and doing
sage i git
should do the job.
It doesn't, because git is already installed. sage f git
doesn't work either. The resulting git still doesn't know about the pull command.
comment:73 Changed 3 years ago by
 Branch changed from public/categories/overabaseringcategory15801 to 15658bd13f22b874423f62b8e6e6fb4d4f7b56ec
 Resolution set to fixed
 Status changed from positive_review to closed
As commented on #10963, I see some questions to be addressed.
If I understand correctly, we sometimes do want to specify the base ring/field, and Volker says that this should be by a "mixin" category (lets call it
FixedBaseRing(...)
) that simply is joined with the nonspecific category, say,Algebras(Fields())
.First complication: We not only have base rings, but (for bimodules) left and right base rings. Should this be reflected in the "FixedBaseBla?" categories as well?
And another complication: We want to have categories expressing the fact that a field (not just a ring) is acting on the objects, and then want to compute the join. Is there an easy programmatic way to let an error be raised when someone tries to compute the join "Algebras(Fields()) & FixedBaseRing?(ZZ)"?