Opened 2 years ago

Closed 3 months ago

#30235 closed enhancement (fixed)

Add construction methods to FiniteRankFreeModule, CombinatorialFreeModule and Cartesian products

Reported by: Matthias Köppe Owned by:
Priority: major Milestone: sage-9.7
Component: linear algebra Keywords:
Cc: Travis Scrimshaw, Eric Gourgoulhon, Frédéric Chapoton, Nicolas M. Thiéry Merged in:
Authors: Matthias Koeppe Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: 3f44174 (Commits, GitHub, GitLab) Commit: 3f4417442c3a8e396e121b054794aa18b4d82154
Dependencies: Stopgaps:

Status badges

Description (last modified by Matthias Köppe)

(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}

Change History (57)

comment:1 Changed 2 years ago by Matthias Köppe

Branch: u/mkoeppe/add_construction_methods_to_finiterankfreemodule_and_combinatorialfreemodule

comment:2 Changed 2 years ago by Matthias Köppe

Commit: 78d063341d140193991bf2f273246d5005c1ea60
Dependencies: #30194

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

comment:3 Changed 2 years ago by Matthias Köppe

Description: modified (diff)

comment:4 Changed 2 years ago by Matthias Köppe

Milestone: sage-9.2sage-9.3

comment:5 Changed 22 months ago by Matthias Köppe

Milestone: sage-9.3sage-9.4

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

comment:6 Changed 17 months ago by Matthias Köppe

Milestone: sage-9.4sage-9.5

comment:7 Changed 12 months ago by Matthias Köppe

Milestone: sage-9.5sage-9.6

comment:8 Changed 8 months ago by Matthias Köppe

Milestone: sage-9.6sage-9.7

comment:9 Changed 4 months ago by Matthias Köppe

Branch: u/mkoeppe/add_construction_methods_to_finiterankfreemodule_and_combinatorialfreemodule
Cc: Frédéric Chapoton added
Commit: 78d063341d140193991bf2f273246d5005c1ea60
Dependencies: #30194

comment:10 Changed 4 months ago by Matthias Köppe

Branch: u/mkoeppe/add_construction_methods_to_finiterankfreemodule_and_combinatorialfreemodule

comment:11 Changed 4 months ago by Matthias Köppe

Authors: Matthias Koeppe
Commit: 33d36751d4c9479db75770107dfea4a3a277863f

New commits:

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

comment:12 Changed 4 months ago by git

Commit: 33d36751d4c9479db75770107dfea4a3a277863f91c56457dd89aebde5f54544ae2ee0a383743b47

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

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

comment:13 Changed 4 months ago by Travis Scrimshaw

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.

comment:14 Changed 4 months ago by git

Commit: 91c56457dd89aebde5f54544ae2ee0a383743b470c8e8ad8c11eef9cc51131f47c5b1e4c3b557fed

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

d1c586bSets.CartesianProducts: Add parent method 'construction'
7014231CombinatorialFreeModule: Fix UniqueRepresentation failure when category is passed
0c8e8adCombinatorialFreeModule.construction: Allow the method from the category to take precedence

comment:15 in reply to:  13 Changed 4 months ago by Matthias Köppe

Replying to tscrim:

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.

Yes, I see a few dozen testsuite failures (_test_construction) in sage.combinat now.

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.

Yes

Last edited 4 months ago by Matthias Köppe (previous) (diff)

comment:16 Changed 4 months ago by Matthias Köppe

Help on these subclasses in sage.combinat is very welcome.

Otherwise I'd just add a lot of def construction(self): return None

comment:17 Changed 4 months ago by Travis Scrimshaw

There are also a number of them in sage.algebra as well. In most cases, the "correct" thing to do would be to have their own ConstructionFunctor. Although we probably would remove a lot of redundant code if we have some generic subclass, say VectorSpaceWithBaseRing from commutative rings that takes a class and arbitrary inputs/keywords except the base ring. It would have a generic merging that would require all of the inputs/keywords to be equal. It might also be possible to pull the key to UniqueRepresentation as these additional structures as a generic implementation in CFM.

comment:18 Changed 4 months ago by Matthias Köppe

Authors: Matthias KoeppeMatthias Koeppe, ...

comment:19 Changed 4 months ago by Matthias Köppe

For a start, I guess we can just give the None construction to all filtered modules

comment:20 Changed 4 months ago by Travis Scrimshaw

True, it makes the situation no worse than before.

comment:21 Changed 4 months ago by git

Commit: 0c8e8ad8c11eef9cc51131f47c5b1e4c3b557fedb26e459646a8920103ba2bf669da439e6e79fef8

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

b26e459FilteredAlgebras, FilteredModules: No construction functor

comment:22 Changed 4 months ago by git

Commit: b26e459646a8920103ba2bf669da439e6e79fef89b66cb5c95508f19c0819d7f8fe25bfc6fa28c98

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

9b66cb5AlgebrasWithBasis: Add construction

comment:23 Changed 4 months ago by Matthias Köppe

Probably safer to just disable the construction for all subclasses

comment:24 Changed 4 months ago by git

Commit: 9b66cb5c95508f19c0819d7f8fe25bfc6fa28c98e0ba7c60d026d62095aaaa72523974dda33f085f

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

e0ba7c6CombinatorialFreeModule: Move method 'contruction' to subclass CombinatorialFreeModule_with_construction

comment:25 Changed 4 months ago by git

Commit: e0ba7c60d026d62095aaaa72523974dda33f085f7897a863fbeb5548dd0e393feeac05ae17c3b0a4

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

7897a86CombinatorialFreeModule: Move method 'contruction' to subclass CombinatorialFreeModule_with_construction

comment:26 in reply to:  17 Changed 4 months ago by Matthias Köppe

Replying to tscrim:

It might also be possible to pull the key to UniqueRepresentation as these additional structures as a generic implementation in CFM.

Yes, I think a solution based on UniqueRepresentation could be a good idea.

comment:27 Changed 4 months ago by git

Commit: 7897a863fbeb5548dd0e393feeac05ae17c3b0a4e07ac34ec08515169cbaae9f202c36ddfc22686b

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

e07ac34CombinatorialFreeModule_with_construction: Add __classcall_private__

comment:28 Changed 4 months ago by git

Commit: e07ac34ec08515169cbaae9f202c36ddfc22686b9d4d1dcb5552be791665272996d42d67425f2314

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

9d4d1dcsage.combinat.all: Import CombinatorialFreeModule_with_construction as CombinatorialFreeModule

comment:29 Changed 4 months ago by Matthias Köppe

sage -t --random-seed=211950378039065916298685714207807222444 doc/en/thematic_tutorials/tutorial-implementing-algebraic-structures.rst  # 1 doctest failed
sage -t --random-seed=211950378039065916298685714207807222444 doc/en/thematic_tutorials/tutorial-objects-and-classes.rst  # 1 doctest failed
sage -t --random-seed=211950378039065916298685714207807222444 sage/combinat/free_module.py  # 1 doctest failed
sage -t --random-seed=211950378039065916298685714207807222444 sage/modules/with_basis/indexed_element.pyx  # 1 doctest failed

comment:30 Changed 4 months ago by git

Commit: 9d4d1dcb5552be791665272996d42d67425f231496f082afafc59ba31abc6b225daf2e43b76a3c20

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

a0177f9Update doctest outputs for new class CombinatorialFreeModule_with_construction
96f082asrc/doc/en/thematic_tutorials/tutorial-implementing-algebraic-structures.rst: In subclassing example, import CombinatorialFreeModule

comment:31 Changed 4 months ago by git

Commit: 96f082afafc59ba31abc6b225daf2e43b76a3c201d4120b04ca5c81bdae95248be5e9217099e31ce

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

1d4120bsrc/sage/tensor/modules/finite_rank_free_module.py: Add construction

comment:32 Changed 4 months ago by git

Commit: 1d4120b04ca5c81bdae95248be5e9217099e31ce61f3ca1884a826bbb67f06e52c26cf781158a7d4

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

61f3ca1src/sage/modules/free_module.py (FreeModule): If with_basis=None, decode rank and start_index from basis_keys

comment:33 Changed 4 months ago by git

Commit: 61f3ca1884a826bbb67f06e52c26cf781158a7d4c9a499c36cdb1d24b5caffc4667e1f42c9c4f01f

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

c9a499csrc/sage/tensor/modules/finite_rank_free_module.py (construction): Support start_index

comment:34 Changed 4 months ago by git

Commit: c9a499c36cdb1d24b5caffc4667e1f42c9c4f01f418be0214175b83a2d7c5b542b60d1bd01a6df75

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

418be02FiniteRankFreeModule.construction, VectorFunctor: Preserve/construct name, latex_name

comment:35 Changed 4 months ago by git

Commit: 418be0214175b83a2d7c5b542b60d1bd01a6df75f5c8afdfdd66ca774529ae3246a635aa211c9e94

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

f5c8afdsrc/sage/tensor/modules/ext_pow_free_module.py: No construction

comment:36 Changed 4 months ago by git

Commit: f5c8afdfdd66ca774529ae3246a635aa211c9e9455606c8158ce69b99cca665eb6f5d4b553f1f082

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

55606c8src/sage/tensor/modules/tensor_free_module.py: No construction

comment:37 Changed 4 months ago by Matthias Köppe

Authors: Matthias Koeppe, ...Matthias Koeppe
Status: newneeds_review

comment:38 Changed 4 months ago by Matthias Köppe

Summary: Add construction methods to FiniteRankFreeModule and CombinatorialFreeModuleAdd construction methods to FiniteRankFreeModule, CombinatorialFreeModule and Cartesian products

comment:39 Changed 4 months ago by Matthias Köppe

sage -t --random-seed=116536317366563428683146425021076892502 src/sage/manifolds/vector_bundle_fiber.py  # 1 doctest failed
sage -t --random-seed=116536317366563428683146425021076892502 src/sage/manifolds/differentiable/tangent_space.py  # 1 doctest failed

comment:40 Changed 4 months ago by git

Commit: 55606c8158ce69b99cca665eb6f5d4b553f1f0821bb9ffa27ce20b7d17cd5808fb768ffcf8f18258

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

1bb9ffaTangentSpace, VectorBundleFiber: No construction

comment:41 Changed 4 months ago by git

Commit: 1bb9ffa27ce20b7d17cd5808fb768ffcf8f182589c299cf7410b40b7e17da194433a0237be5bf9be

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

9c299cfsrc/sage/categories/pushout.py: Expand doctest

comment:42 Changed 4 months ago by Matthias Köppe

Description: modified (diff)

comment:43 Changed 4 months ago by Matthias Köppe

Description: modified (diff)

comment:44 Changed 4 months ago by git

Commit: 9c299cf7410b40b7e17da194433a0237be5bf9be2c6f7ee2dbc40bbdc599da9f4375d3533dd89b5f

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

2c6f7eesrc/sage/categories/pushout.py: Fix pycodestyle

comment:45 Changed 4 months ago by Matthias Köppe

Tests pass, the pyright crash is unrelated; needs review

comment:46 Changed 4 months ago by Travis Scrimshaw

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.

comment:47 Changed 4 months ago by Matthias Köppe

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

comment:48 in reply to:  47 Changed 4 months ago by Travis Scrimshaw

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).

comment:49 Changed 4 months ago by Matthias Köppe

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

comment:50 Changed 4 months ago by Matthias Köppe

I'm making this change now.

comment:51 Changed 3 months ago by git

Commit: 2c6f7ee2dbc40bbdc599da9f4375d3533dd89b5fa81428a82f09fa6abf9793151700f58ea903ced2

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

comment:52 Changed 3 months ago by git

Commit: a81428a82f09fa6abf9793151700f58ea903ced23f4417442c3a8e396e121b054794aa18b4d82154

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

comment:53 Changed 3 months ago by Travis Scrimshaw

Cc: Nicolas M. Thiéry added
Reviewers: Travis Scrimshaw

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

comment:54 in reply to:  53 Changed 3 months ago by Eric Gourgoulhon

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 ;-)

comment:55 Changed 3 months ago by Travis Scrimshaw

Status: needs_reviewpositive_review

Then let it be so.

comment:56 Changed 3 months ago by Matthias Köppe

Thanks!

comment:57 Changed 3 months ago by Volker Braun

Branch: u/mkoeppe/add_construction_methods_to_finiterankfreemodule_and_combinatorialfreemodule3f4417442c3a8e396e121b054794aa18b4d82154
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.