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

Add construction methods to FiniteRankFreeModule, CombinatorialFreeModule and Cartesian products #30235

Closed
mkoeppe opened this issue Jul 28, 2020 · 79 comments

Comments

@mkoeppe
Copy link
Contributor

mkoeppe commented Jul 28, 2020

(follow-up from #30194)

... extending sage.categories.pushout.VectorFunctor

for example:

            sage: M = FreeModule(ZZ, 4, with_basis=None, name='M')
            sage: latex(M)
            M
            sage: from sage.categories.pushout import VectorFunctor, pushout
            sage: M_QQ = pushout(M, QQ)
            sage: latex(M_QQ)
            M \otimes \Bold{Q}

CC: @tscrim @egourgoulhon @fchapoton @nthiery

Component: linear algebra

Author: Matthias Koeppe

Branch/Commit: 3f44174

Reviewer: Travis Scrimshaw

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

@mkoeppe mkoeppe added this to the sage-9.2 milestone Jul 28, 2020
@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jul 28, 2020

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jul 28, 2020

New commits:

9ee2a48Create CombinatorialFreeModule if basis keys given
d08ee98zero_vector: Handle bad input like zero_vector(3.6) before passing it to ZZ.__pow__
5d56004FreeModule: SImplify argument handling
9116a1csrc/sage/misc/sageinspect.py: Do not use FreeModule as an example for a class instance
20b10f0Extend VectorSpace factory in the same way
6af7fa4Move documentation from FreeModuleFactory to FreeModule
5910876FreeModule: Add to documentation
78d0633More documentation

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jul 28, 2020

Commit: 78d0633

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jul 28, 2020

Dependencies: #30194

@mkoeppe

This comment has been minimized.

@mkoeppe mkoeppe modified the milestones: sage-9.2, sage-9.3 Aug 13, 2020
@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 13, 2021

comment:5

Setting new milestone based on a cursory review of ticket status, priority, and last modification date.

@mkoeppe mkoeppe modified the milestones: sage-9.3, sage-9.4 Feb 13, 2021
@mkoeppe mkoeppe modified the milestones: sage-9.4, sage-9.5 Jul 19, 2021
@mkoeppe mkoeppe modified the milestones: sage-9.5, sage-9.6 Dec 14, 2021
@mkoeppe mkoeppe modified the milestones: sage-9.6, sage-9.7 Apr 1, 2022
@mkoeppe
Copy link
Contributor Author

mkoeppe commented Aug 21, 2022

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Aug 21, 2022

Changed commit from 78d0633 to none

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Aug 21, 2022

Changed dependencies from #30194 to none

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Aug 21, 2022

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Aug 21, 2022

New commits:

33d3675src/sage/categories/pushout.py (VectorFunctor): Add support for other keywords of the FreeModule constructor

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Aug 21, 2022

Author: Matthias Koeppe

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Aug 21, 2022

Commit: 33d3675

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 21, 2022

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

91c5645src/sage/combinat/free_module.py (CombinatorialFreeModule): Add construction method

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 21, 2022

Changed commit from 33d3675 to 91c5645

@tscrim
Copy link
Collaborator

tscrim commented Aug 22, 2022

comment:13

We will need to be really careful with this because lots of classes are subclasses of CFM, where the result of construction() will become wrong if we do the generic one.

However, I am generally +1 on doing this. There is a ticket out there on implementing extension of scalars, and this would be a step towards doing that generically by being able to implement pushouts.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 23, 2022

Changed commit from 9c299cf to 2c6f7ee

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Aug 23, 2022

comment:45

Tests pass, the pyright crash is unrelated; needs review

@tscrim
Copy link
Collaborator

tscrim commented Aug 24, 2022

comment:46

While the subclass for the construction version is the proper way to do it, I am not sure how much of a hassle it will create long-term once we have a more general way to do constructions for things that subclass CFM. The hack way would be

def construction(self):
    if self.__class__.__mro__[1] == CombinatorialFreeModule:
         return VectorFunctor(...)
    return super().construction()

(We cannot to type(self) == CFM because of the dynamic class created from the category. There is a function or method to get this more robustly, but I don't remember offhand.) I am fairly convinced that your proposal is how we should go, but I just have this little nagging doubt.

One thing that does need to be changed is that we will have two CFM classes with (almost) identical (public) documentations. This is easy to fix by adding a class-level doc to the new class explaining the difference.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Aug 24, 2022

comment:47

Another hackish way would be to assign self.construction in a __classcall_private__ method.

@tscrim
Copy link
Collaborator

tscrim commented Aug 24, 2022

comment:48

Replying to @mkoeppe:

Another hackish way would be to assign self.construction in a __classcall_private__ method.

Indeed, although I think that is even more of a hack since it is doing stuff non-locally (to the method).

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Aug 24, 2022

comment:49

I don't have an objection to using self.__class__.__base__ or something like this like you suggested. It's probably less confusing than the renaming reimport

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Aug 24, 2022

comment:50

I'm making this change now.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 24, 2022

Changed commit from 2c6f7ee to a81428a

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 24, 2022

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

165e6ffRevert "src/doc/en/thematic_tutorials/tutorial-implementing-algebraic-structures.rst: In subclassing example, import CombinatorialFreeModule"
c95e97fRevert "Update doctest outputs for new class CombinatorialFreeModule_with_construction"
f22988fRevert "sage.combinat.all: Import CombinatorialFreeModule_with_construction as CombinatorialFreeModule"
9a2c87eRevert "CombinatorialFreeModule_with_construction: Add __classcall_private__"
d78a002Revert "CombinatorialFreeModule: Move method 'contruction' to subclass CombinatorialFreeModule_with_construction"
a81428aCombinatorialFreeModule.construction: On subclasses, return None

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 24, 2022

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

3f44174src/sage/tensor/modules/finite_rank_free_module.py, src/sage/categories/pushout.py: Fix name mappings

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 24, 2022

Changed commit from a81428a to 3f44174

@tscrim
Copy link
Collaborator

tscrim commented Aug 25, 2022

Reviewer: Travis Scrimshaw

@tscrim
Copy link
Collaborator

tscrim commented Aug 25, 2022

comment:53

I think this is acceptable. Éric, do you have any comments?

@egourgoulhon
Copy link
Member

comment:54

Replying to @tscrim:

I think this is acceptable. Éric, do you have any comments?

No, I don't. This is mostly beyond my knowledge, but thanks for asking ;-)

@tscrim
Copy link
Collaborator

tscrim commented Aug 27, 2022

comment:55

Then let it be so.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Aug 27, 2022

comment:56

Thanks!

@vbraun
Copy link
Member

vbraun commented Aug 29, 2022

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

4 participants