Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement generic support for parents with (multiple) realizations #7980

Closed
jbandlow opened this issue Jan 18, 2010 · 28 comments
Closed

Implement generic support for parents with (multiple) realizations #7980

jbandlow opened this issue Jan 18, 2010 · 28 comments

Comments

@jbandlow
Copy link

This patch implement generic support for parents with (multiple) realizations
See:

    Sets().WithRealizations().example()

Depends on #12484
Depends on #12464

CC: @sagetrac-sage-combinat

Component: categories

Keywords: Cernay2012

Author: Nicolas M. Thiéry

Reviewer: Simon King, Florent Hivert

Merged: sage-5.0.beta11

Issue created by migration from https://trac.sagemath.org/ticket/7980

@jbandlow

This comment has been minimized.

@mwhansen
Copy link
Contributor

mwhansen commented May 7, 2010

comment:2

Didn't we want Realization rather than Representation?

@nthiery

This comment has been minimized.

@nthiery nthiery changed the title Move 'Category with concrete representation' code into Sage Implement generic support for parents with (multiple) realizations May 7, 2010
@nthiery
Copy link
Contributor

nthiery commented Jan 13, 2011

Author: Nicolas M. Thiéry

@nthiery

This comment has been minimized.

@simon-king-jena
Copy link
Member

comment:5

Since the ticket description is rather sparse, I'm collecting here some statements that emerged from reading the patch and discussing with Nicolas, who is sitting beside me.

  • It disposes of abstract categories. Hooray!
  • On the one hand, one has (abstract) vectorspaces. A realization of such abstract vector space is given by specific data, for example by a basis, but it should be emphasized that not all realizations of a vector space are necessarily in terms of a basis. So, not any realization of a vectorspace is a "vectorspace with basis".
  • The realizations of a vector space are related by canonical morphisms (for example, change of basis).
  • Is any "vectorspace with basis" a realisation of a vector space? No! There is no gurantee that this specific vectorspace with basis is equiped with the above-mentioned canonical morphisms.

TODO

  • The docstring of the class Category_realization has no examples. Its __init__ method should define its arguments and provide a test. _get_name is not documented either.

  • Similarly, Realizations is undocumented.

  • Realizations.super_categories() returns [Sets()]. Idea of Nicolas: Add an optional argument Realizations(parent, category=Bla), so that the default for Bla is Sets(). The given category will be returned as super category.

  • I tried some example, with comments/questions inserted:

sage: C = Algebras(QQ)
sage: C.WithRealizations()
Join of Category of algebras over Rational Field and Category of sets with realizations

Does it really have to be a join category?

sage: from sage.categories.category_types import Category_realization
sage: Category_realization(QQ)
The category of category_realization of Rational Field

So, do I understand correctly that the set of all realisations of QQ forms a category, each object being a realisation of QQ, the morphisms being canonical maps between the different realisations?

sage: Category_realization(QQ).is_subcategory(Rings().WithRealizations())
BOOM
sage: hasattr(Category_realization(QQ), 'super_categories')
False

So, that's a bug. Category_realization(QQ) is a category and thus MUST have a super_categories method.

Is the category of "Bla with realisations" a 2-category?

Since Nicolas is now teaching, I can not ask him directly:

  • If I understand correctly, for any parent P there is a category Category_realizations(P).
  • For any category C, there is a category C.WithRealizations().
  • Shouldn't it be the case that for any P in C, the realisations of P (thus, the category Category_realizations(P)) is an object in C.WithRealizations()?
  • But if the objects of C.WithRealizations() are categories themselves, shouldn't we first implement the notion of higher categories in Sage, before we treat the category of realizations of objects of C?

@simon-king-jena
Copy link
Member

Work Issues: Implement super_categories for Category_realizations. Wait for 2-categories to be implemented?

@simon-king-jena
Copy link
Member

Changed work issues from Implement super_categories for Category_realizations. Wait for 2-categories to be implemented? to Implement super_categories for Category_realizations.

@simon-king-jena
Copy link
Member

comment:6

Replying to @simon-king-jena:

Is the category of "Bla with realisations" a 2-category?

No, it isn't. I had a totally wrong notion of a higher category. Sorry.

Anyway, the missing super_categories method must be provided (as for any category).

@nthiery
Copy link
Contributor

nthiery commented Feb 9, 2012

Dependencies: #12484, #12464

@nthiery
Copy link
Contributor

nthiery commented Feb 9, 2012

Changed keywords from none to Cernay2012

@nthiery
Copy link
Contributor

nthiery commented Feb 18, 2012

comment:8

100% doctest coverage; all test pass

Category_realization (now Category_realization_of_parent) does not need a super_category method: it is an abstract class.

@nthiery
Copy link
Contributor

nthiery commented Feb 18, 2012

Reviewer: Simon King, Florent Hivert

@nthiery

This comment has been minimized.

@nthiery
Copy link
Contributor

nthiery commented Feb 18, 2012

Changed work issues from Implement super_categories for Category_realizations. to none

@hivert
Copy link

hivert commented Feb 20, 2012

comment:9

Hi Nicolas,

I just posted a
review patch
on sage-combinat queue. Here are the main changes

  1. This is mostly documentation, except that I changed trivially the code in
    the example, by renaming some parameter names:
  • in SubsetAlgebra(S) the parameter S is a set;

  • in Fundamental(S), In(S) and Out(S) it is a
    SubsetAlgebra;

I was really confused about that reading the code so that I renamed S
to SAlg in the second case and added an INPUT: field in the doc

  1. The correct markup for see also is the following:
.. seealso::
   bla bla bla
   bla bla 

as variants, you can use uppercase as Sphinx markup is not case sensitive. You
can also put everything on one line if it fits:

.. SEEALSO:: bla bla bla

If you put a space as in ..see also:: then this become a comment and is
ignored by Sphinx ! This remind me that I should finish #12078 Add an example
of SEE ALSO

  1. There is a missing doctest in my review patch
    sage: A.F() in A.Realizations()
Expected:
    True
Got:
    False

I left it on purpose. I realize that the correct doctest is

    sage: A.F() in A.Bases()
Expected:
    True
Got:
    False

But it took me quite a lot of thinking understanding that. I think we should
explain this somewhere, probably where the broken test is.

Please review my numerous doc change, fold the patch if you are Ok with it.

Florent

@nthiery
Copy link
Contributor

nthiery commented Feb 20, 2012

comment:10

Replying to @hivert:

I just posted a review patch on sage-combinat queue.

Thanks!

I went through it, folded it in, and added a
[http://combinat.sagemath.org/patches/file/tip/trac_7980-multiple-realizations-review-nt.patch
review review patch]

  1. This is mostly documentation, except that I changed trivially the code in
    the example, by renaming some parameter names:
  • in SubsetAlgebra(S) the parameter S is a set;

  • in Fundamental(S), In(S) and Out(S) it is a
    SubsetAlgebra;

I was really confused about that reading the code so that I renamed S
to SAlg in the second case and added an INPUT: field in the doc

Good point. I ended up renaming SAlg to A since this is what we use in
all the examples.

  1. There is a missing doctest in my review patch
    sage: A.F() in A.Realizations()
Expected:
    True
Got:
    False

Ouch! That's not good. I am not sure this is the best approach, but I
added A.Realizations() to the super categories Bases(A). Maybe
A.Realizations() should be Bases(A) instead. In any cases, that will
work for now. I added A._test_realizations() which checks:

    R in A.Realizations() for R in A.realizations()

Feel free to fold in my review patch and repost after checking it out.

Cheers,
Nicolas

@nthiery
Copy link
Contributor

nthiery commented Feb 21, 2012

comment:11

Please update the queue first, since I took off the fix to the CategoryObject link to put it in #9469

@nthiery
Copy link
Contributor

nthiery commented Feb 25, 2012

comment:12

Replying to @nthiery:

Please update the queue first, since I took off the fix to the CategoryObject link to put it in #9469

Again: I fixed a couple trivially failing doctests.

@simon-king-jena
Copy link
Member

Work Issues: Post the patches on trac

@simon-king-jena
Copy link
Member

Replying to @jbandlow:

This patch implement generic support for parents with (multiple) realizations

What patch? This ticket does not provide any patch.

@nthiery
Copy link
Contributor

nthiery commented Mar 10, 2012

comment:14

Replying to @simon-king-jena:

Replying to @jbandlow:

This patch implement generic support for parents with (multiple) realizations

What patch? This ticket does not provide any patch.

I should have mentioned that it is in on the Sage-Combinat queue, and the review by Florent is occuring here. In such situations it's a waste of time to update the patch on trac all the time; and leaving an outdated patch on trac is not so good either (someone might waste time reviewing old stuff).

@hivert
Copy link

hivert commented Mar 15, 2012

comment:15

For the record: as of version a7993225ce8e/trac_7980-multiple-realizations-nt.patch I'm Ok with the patch upto a few documentation glitches. I pushed the following patch a7993225ce8e/trac_7980-multiple-realizations-review-fh.patch. If you are Ok with my review patch and the tests pass you can set positive review on my behalf. If so don't forget to fold and update.

Florent

@nthiery
Copy link
Contributor

nthiery commented Mar 16, 2012

@nthiery
Copy link
Contributor

nthiery commented Mar 16, 2012

comment:16

I folded Florent's patch, and we fixed together a couple remaining glitches on the Sage-Combinat queue.

Florent: the sphinx errors were clearly due to a broken sphinx state on my test machine. After nuking the doctrees, this compiled smoothly. So I set a positive review on your behalf.

@hivert
Copy link

hivert commented Mar 16, 2012

comment:17

One more ! Good !

Florent

@jdemeyer
Copy link

Changed work issues from Post the patches on trac to none

@jdemeyer
Copy link

Merged: sage-5.0.beta11

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants