Opened 12 months ago
Closed 8 months ago
#15801 closed enhancement (fixed)
Categories over a base ring category
Reported by: | nthiery | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-6.3 |
Component: | categories | Keywords: | |
Cc: | sage-combinat, 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 tscrim)
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 follow-up tickets would be to:
- make Modules(...) into a covariant functorial constructions. This would give a proper framework for the feature Modules(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 12 months ago by SimonKing
comment:2 Changed 12 months ago by nthiery
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 super-categories "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 follow-up: ↓ 4 Changed 12 months ago by SimonKing
OK, that's the other possibility: Rather than having a separate mix-in 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 super-categories of Algebras(R). But if C is a category, do we want that Algebras(D) is among the super-categories 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 cmp-key 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 super-categories of a category with fixed base ring should all be categories with unspecified base rings. Hence, Algebras(R) should not have the super-categories Rings() and Modules(R), but Algebras(R.category()).
comment:4 in reply to: ↑ 3 ; follow-up: ↓ 5 Changed 12 months ago by nthiery
Replying to SimonKing:
OK, that's the other possibility: Rather than having a separate mix-in 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 super-categories of Algebras(R). But if C is a category, do we want that Algebras(D) is among the super-categories 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 cmp-key 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 super-categories of a category with fixed base ring should all be categories with unspecified base rings. Hence, Algebras(R) should not have the super-categories Rings() and Modules(R), but Algebras(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 ; follow-up: ↓ 7 Changed 12 months ago by SimonKing
- 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 super-categories of a category with fixed base ring should all be categories with unspecified base rings. Hence, Algebras(R) should not have the super-categories Rings() and Modules(R), but Algebras(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 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
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().
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 of Algebras(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 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>
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 12 months ago by SimonKing
- Dependencies changed from 10963 to #10963
comment:7 in reply to: ↑ 5 Changed 12 months ago by nthiery
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 of Algebras(Fields()) from Algebras(Rings()), but probably not from Algebras(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)) and Modules(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 12 months ago by nbruin
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 follow-up: ↓ 13 Changed 11 months ago by 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)
comment:10 Changed 10 months ago by nthiery
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 time-performance issue for polynomials is indeed fixed for polynomials :-)
Now, on to matrices!
comment:11 Changed 10 months ago by nthiery
- Branch set to refs/heads/public/categories/over-a-base-ring-category-15801
comment:12 Changed 10 months ago by nthiery
- Branch changed from refs/heads/public/categories/over-a-base-ring-category-15801 to public/categories/over-a-base-ring-category-15801
comment:13 in reply to: ↑ 9 Changed 10 months ago by nthiery
- 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/10963-doc-distributive' of trac.sagemath.org:sage into public/ticket/10963-doc-distributive |
28b39a2 | Merge branch 'develop' into public/ticket/10963-doc-distributive |
70972db | Merge branch 'develop' into public/ticket/10963-doc-distributive |
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/axioms-10963' into categories/over_a_base_category-15801 |
3a63847 | Trac 15801: updated doctest |
comment:14 Changed 10 months ago by nthiery
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 10 months ago by nthiery
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 10 months ago by git
- 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/axioms-10963' into categories/over_a_base_category-15801 |
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 10 months ago by nthiery
Up to merging in #15919, it's likely that all long test will pass. Running them now.
comment:18 Changed 10 months ago by git
- 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 follow-up: ↓ 20 Changed 10 months ago by tscrim
- 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 9 months ago by nthiery
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 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?
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 follow-up: ↓ 23 Changed 9 months ago by tscrim
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 9 months ago by git
- Commit changed from 12afbe46ebafe56cf74661bb6d5ea81af583ba83 to 0397865de0a1b9636b7501f0d666f08473b148fb
Branch pushed to git repo; I updated commit sha1. New commits:
8eaaa99 | Merge branch 'develop' into public/ticket/10963-doc-distributive |
feab04a | manual merge with 6.2.beta8 |
ce2193e | Merge branch 'public/ticket/10963-doc-distributive' of trac.sagemath.org:sage into public/ticket/10963-doc-distributive |
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 ; follow-up: ↓ 25 Changed 9 months ago by nthiery
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 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).
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 R-modules, and M' and N' as S-modules, 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 9 months ago by nthiery
- Status changed from new to needs_review
comment:25 in reply to: ↑ 23 ; follow-up: ↓ 27 Changed 9 months ago by 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] 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 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>
<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 R-modules, and M' and N' as S-modules, 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 S-modules. 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 9 months ago by git
- 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/axioms-10963 |
d396719 | Merge branch '6.2.rc0' and categories/axioms-10963 into categories/over_a_base_category-15801 |
comment:27 in reply to: ↑ 25 Changed 9 months ago by nthiery
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] where G,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 9 months ago by git
- 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/10963-doc-distributive' into public/categories/over-a-base-ring-category-15801 |
5b8438e | Fixing up doc and some other minor things. |
6a960b2 | Fixed Sets.Algebras(). |
comment:29 follow-up: ↓ 31 Changed 9 months ago by 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 (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 9 months ago by git
- 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/axioms-10963' into categories/over_a_base_category-15801 |
1f09b8b | Trac 15801: proofread Travis's comments + minor doctest improvements |
comment:31 in reply to: ↑ 29 Changed 9 months ago by nthiery
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 (so GroupAlgebras(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 9 months ago by tscrim
If all doctests pass for you, then positive review for this.
comment:33 Changed 9 months ago by nthiery
- 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 9 months ago by git
- Commit changed from 1f09b8bde256c00bab2359e799b88e9da2273720 to 2114d54f98b80c9542b61051600d9ff686f99f13
Branch pushed to git repo; I updated commit sha1. Last 10 new commits:
55a36d6 | post-apocalyptic edits (correct this time) |
72663b7 | Merge branch 'public/ticket/10963-doc-distributive' of trac.sagemath.org:sage into public/ticket/10963-doc-distributive |
67cbc90 | Merge branch 'public/ticket/10963-doc-distributive' into public/categories/over-a-base-ring-category-15801 |
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/10963-doc-distributive' into public/categories/over-a-base-ring-category-15801 |
ac91ed0 | Added _Hom_() method for Modules. |
2114d54 | Merge branch 'public/categories/over-a-base-ring-category-15801' of trac.sagemath.org:sage into public/categories/over-a-base-ring-category-15801 |
comment:35 Changed 9 months ago by tscrim
- 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 9 months ago by nthiery
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 context-wise (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 9 months ago by nthiery
- 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 follow-up: ↓ 39 Changed 9 months ago by 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 the Parent.__init__). So I'd say my _Hom_ and #16275 should coexist in the final version.
comment:39 in reply to: ↑ 38 Changed 9 months ago by nthiery
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 the Parent.__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 9 months ago by git
- 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_category-15801 |
fe1c1c5 | Trac 10963: Removed the old _join helper function in category.py (why was it still there???) |
d6dc1fd | Merge branch 'categories/axioms-10963' into categories/over_a_base_category-15801 |
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 9 months ago by nthiery
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_category-15801 |
fe1c1c5 | Trac 10963: Removed the old _join helper function in category.py (why was it still there???) |
d6dc1fd | Merge branch 'categories/axioms-10963' into categories/over_a_base_category-15801 |
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 9 months ago by tscrim
- 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 9 months ago by vbraun_spam
- Milestone changed from sage-6.2 to sage-6.3
comment:44 Changed 9 months ago by git
- 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/10963-doc-distributive' 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/10963-doc-distributive' into categories/axioms-10963 |
f22508b | Trac 10963: proofread Darij's edits |
8da7522 | Trac 10963: degraded a bit a newly added cross-link to work around 'make doc-clean; make clean' failure |
4ef8b27 | Merge branch 'public/ticket/10963-doc-distributive' of git://trac.sagemath.org/sage into 10963new |
afb911c | corrected conflict resolution |
82c8ee0 | Merge branch 'public/ticket/10963-doc-distributive' of trac.sagemath.org:sage into public/ticket/10963-doc-distributive |
79f4766 | Merge branch 'public/categories/over-a-base-ring-category-15801' of trac.sagemath.org:sage into public/categories/over-a-base-ring-category-15801 |
comment:45 Changed 9 months ago by tscrim
Rebased over rebased #10963.
comment:46 Changed 9 months ago by tscrim
- Status changed from needs_review to positive_review
comment:47 Changed 9 months ago by vbraun
- 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 9 months ago by nthiery
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 9 months ago by vbraun
No, #14102 wasn't applied since it depends on this ticket.
comment:50 Changed 9 months ago by git
- 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 9 months ago by nthiery
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 cherry-picked back 1579b88 to this branch. This should work now. Running the tests.
Thanks for your report!
Cheers,
Nicolas
comment:52 Changed 9 months ago by nthiery
- Status changed from needs_work to positive_review
comment:53 Changed 9 months ago by vbraun
- Status changed from positive_review to needs_work
- Work issues set to doc-pdf
make doc-pdf fails (probably due to #10963)
comment:54 follow-up: ↓ 55 Changed 9 months ago by vbraun
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 9 months ago by nthiery
comment:56 Changed 9 months ago by git
- Commit changed from 281f5392e81e98d835ddf249325e66c1c36a08cc to dcb11d850530d23e1d8867daa84ade0e04d7ce15
Branch pushed to git repo; I updated commit sha1. New commits:
96c631f | Merge branch 'develop' into categories/axioms-10963 |
b1a2aed | Trac 10963: two typo fixes to let the pdf documentation compile |
c16f18b | Merge branch 'public/ticket/10963-doc-distributive' of trac.sagemath.org:sage into categories/axioms-10963 |
dcb11d8 | Merge branch 'categories/axioms-10963' into categories/over_a_base_category-15801 |
comment:57 Changed 9 months ago by nthiery
Pdf compilation fixed in #10963 and merged here.
comment:58 follow-ups: ↓ 59 ↓ 61 Changed 9 months ago by SimonKing
- 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 9 months ago by nthiery
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 in Modules(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 9 months ago by git
- 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 9 months ago by nthiery
Replying to SimonKing:
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.
Done. Please review! Running all long tests now.
comment:62 Changed 9 months ago by SimonKing
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 9 months ago by SimonKing
I read the latest commit, and it seems good. Hence, I wait for the test results...
comment:64 Changed 9 months ago by SimonKing
The pdf does build...
comment:65 Changed 9 months ago by SimonKing
- Work issues doc-pdf 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 non-zero exit code (1). This happened while executing "git -c user.email=doc@test.test -c user.name=doctest pull /home/king/.sage/temp/linux-etl7.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 9 months ago by SimonKing
Is this perhaps an issue of sage-6.3.beta0?
comment:67 follow-up: ↓ 72 Changed 9 months ago by nthiery
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 follow-up: ↓ 71 Changed 9 months ago by darij
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 follow-up: ↓ 70 Changed 9 months ago by tscrim
- 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 system-wide git.
comment:70 in reply to: ↑ 69 Changed 9 months ago by SimonKing
Replying to tscrim:
I also only use my system-wide git.
Are you sure that you use your system-wide git when you run the tests in sage.dev? Anyway, my system-wide git does know about git pull---only Sage's git doesn't know.
Best regards,
Simon
comment:71 in reply to: ↑ 68 Changed 9 months ago by SimonKing
Replying to darij:
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...
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 9 months ago by SimonKing
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 8 months ago by vbraun
- Branch changed from public/categories/over-a-base-ring-category-15801 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 non-specific 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)"?