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:  sage9.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: 
Description (last modified by )
(followup 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
Branch:  → u/mkoeppe/add_construction_methods_to_finiterankfreemodule_and_combinatorialfreemodule 

comment:2 Changed 2 years ago by
Commit:  → 78d063341d140193991bf2f273246d5005c1ea60 

Dependencies:  → #30194 
comment:3 Changed 2 years ago by
Description:  modified (diff) 

comment:4 Changed 2 years ago by
Milestone:  sage9.2 → sage9.3 

comment:5 Changed 22 months ago by
Milestone:  sage9.3 → sage9.4 

Setting new milestone based on a cursory review of ticket status, priority, and last modification date.
comment:6 Changed 17 months ago by
Milestone:  sage9.4 → sage9.5 

comment:7 Changed 12 months ago by
Milestone:  sage9.5 → sage9.6 

comment:8 Changed 8 months ago by
Milestone:  sage9.6 → sage9.7 

comment:9 Changed 4 months ago by
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
Branch:  → u/mkoeppe/add_construction_methods_to_finiterankfreemodule_and_combinatorialfreemodule 

comment:11 Changed 4 months ago by
Authors:  → Matthias Koeppe 

Commit:  → 33d36751d4c9479db75770107dfea4a3a277863f 
New commits:
33d3675  src/sage/categories/pushout.py (VectorFunctor): Add support for other keywords of the FreeModule constructor

comment:12 Changed 4 months ago by
Commit:  33d36751d4c9479db75770107dfea4a3a277863f → 91c56457dd89aebde5f54544ae2ee0a383743b47 

Branch pushed to git repo; I updated commit sha1. New commits:
91c5645  src/sage/combinat/free_module.py (CombinatorialFreeModule): Add construction method

comment:13 followup: 15 Changed 4 months ago by
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
Commit:  91c56457dd89aebde5f54544ae2ee0a383743b47 → 0c8e8ad8c11eef9cc51131f47c5b1e4c3b557fed 

Branch pushed to git repo; I updated commit sha1. New commits:
d1c586b  Sets.CartesianProducts: Add parent method 'construction'

7014231  CombinatorialFreeModule: Fix UniqueRepresentation failure when category is passed

0c8e8ad  CombinatorialFreeModule.construction: Allow the method from the category to take precedence

comment:15 Changed 4 months ago by
Replying to tscrim:
We will need to be really careful with this because lots of classes are subclasses of
CFM
, where the result ofconstruction()
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
comment:16 Changed 4 months ago by
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 followup: 26 Changed 4 months ago by
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
Authors:  Matthias Koeppe → Matthias Koeppe, ... 

comment:19 Changed 4 months ago by
For a start, I guess we can just give the None
construction to all filtered modules
comment:21 Changed 4 months ago by
Commit:  0c8e8ad8c11eef9cc51131f47c5b1e4c3b557fed → b26e459646a8920103ba2bf669da439e6e79fef8 

Branch pushed to git repo; I updated commit sha1. New commits:
b26e459  FilteredAlgebras, FilteredModules: No construction functor

comment:22 Changed 4 months ago by
Commit:  b26e459646a8920103ba2bf669da439e6e79fef8 → 9b66cb5c95508f19c0819d7f8fe25bfc6fa28c98 

Branch pushed to git repo; I updated commit sha1. New commits:
9b66cb5  AlgebrasWithBasis: Add construction

comment:23 Changed 4 months ago by
Probably safer to just disable the construction
for all subclasses
comment:24 Changed 4 months ago by
Commit:  9b66cb5c95508f19c0819d7f8fe25bfc6fa28c98 → e0ba7c60d026d62095aaaa72523974dda33f085f 

Branch pushed to git repo; I updated commit sha1. New commits:
e0ba7c6  CombinatorialFreeModule: Move method 'contruction' to subclass CombinatorialFreeModule_with_construction

comment:25 Changed 4 months ago by
Commit:  e0ba7c60d026d62095aaaa72523974dda33f085f → 7897a863fbeb5548dd0e393feeac05ae17c3b0a4 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
7897a86  CombinatorialFreeModule: Move method 'contruction' to subclass CombinatorialFreeModule_with_construction

comment:26 Changed 4 months ago by
Replying to tscrim:
It might also be possible to pull the key to
UniqueRepresentation
as these additional structures as a generic implementation inCFM
.
Yes, I think a solution based on UniqueRepresentation
could be a good idea.
comment:27 Changed 4 months ago by
Commit:  7897a863fbeb5548dd0e393feeac05ae17c3b0a4 → e07ac34ec08515169cbaae9f202c36ddfc22686b 

Branch pushed to git repo; I updated commit sha1. New commits:
e07ac34  CombinatorialFreeModule_with_construction: Add __classcall_private__

comment:28 Changed 4 months ago by
Commit:  e07ac34ec08515169cbaae9f202c36ddfc22686b → 9d4d1dcb5552be791665272996d42d67425f2314 

Branch pushed to git repo; I updated commit sha1. New commits:
9d4d1dc  sage.combinat.all: Import CombinatorialFreeModule_with_construction as CombinatorialFreeModule

comment:29 Changed 4 months ago by
sage t randomseed=211950378039065916298685714207807222444 doc/en/thematic_tutorials/tutorialimplementingalgebraicstructures.rst # 1 doctest failed sage t randomseed=211950378039065916298685714207807222444 doc/en/thematic_tutorials/tutorialobjectsandclasses.rst # 1 doctest failed sage t randomseed=211950378039065916298685714207807222444 sage/combinat/free_module.py # 1 doctest failed sage t randomseed=211950378039065916298685714207807222444 sage/modules/with_basis/indexed_element.pyx # 1 doctest failed
comment:30 Changed 4 months ago by
Commit:  9d4d1dcb5552be791665272996d42d67425f2314 → 96f082afafc59ba31abc6b225daf2e43b76a3c20 

comment:31 Changed 4 months ago by
Commit:  96f082afafc59ba31abc6b225daf2e43b76a3c20 → 1d4120b04ca5c81bdae95248be5e9217099e31ce 

Branch pushed to git repo; I updated commit sha1. New commits:
1d4120b  src/sage/tensor/modules/finite_rank_free_module.py: Add construction

comment:32 Changed 4 months ago by
Commit:  1d4120b04ca5c81bdae95248be5e9217099e31ce → 61f3ca1884a826bbb67f06e52c26cf781158a7d4 

Branch pushed to git repo; I updated commit sha1. New commits:
61f3ca1  src/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
Commit:  61f3ca1884a826bbb67f06e52c26cf781158a7d4 → c9a499c36cdb1d24b5caffc4667e1f42c9c4f01f 

Branch pushed to git repo; I updated commit sha1. New commits:
c9a499c  src/sage/tensor/modules/finite_rank_free_module.py (construction): Support start_index

comment:34 Changed 4 months ago by
Commit:  c9a499c36cdb1d24b5caffc4667e1f42c9c4f01f → 418be0214175b83a2d7c5b542b60d1bd01a6df75 

Branch pushed to git repo; I updated commit sha1. New commits:
418be02  FiniteRankFreeModule.construction, VectorFunctor: Preserve/construct name, latex_name

comment:35 Changed 4 months ago by
Commit:  418be0214175b83a2d7c5b542b60d1bd01a6df75 → f5c8afdfdd66ca774529ae3246a635aa211c9e94 

Branch pushed to git repo; I updated commit sha1. New commits:
f5c8afd  src/sage/tensor/modules/ext_pow_free_module.py: No construction

comment:36 Changed 4 months ago by
Commit:  f5c8afdfdd66ca774529ae3246a635aa211c9e94 → 55606c8158ce69b99cca665eb6f5d4b553f1f082 

Branch pushed to git repo; I updated commit sha1. New commits:
55606c8  src/sage/tensor/modules/tensor_free_module.py: No construction

comment:37 Changed 4 months ago by
Authors:  Matthias Koeppe, ... → Matthias Koeppe 

Status:  new → needs_review 
comment:38 Changed 4 months ago by
Summary:  Add construction methods to FiniteRankFreeModule and CombinatorialFreeModule → Add construction methods to FiniteRankFreeModule, CombinatorialFreeModule and Cartesian products 

comment:39 Changed 4 months ago by
sage t randomseed=116536317366563428683146425021076892502 src/sage/manifolds/vector_bundle_fiber.py # 1 doctest failed sage t randomseed=116536317366563428683146425021076892502 src/sage/manifolds/differentiable/tangent_space.py # 1 doctest failed
comment:40 Changed 4 months ago by
Commit:  55606c8158ce69b99cca665eb6f5d4b553f1f082 → 1bb9ffa27ce20b7d17cd5808fb768ffcf8f18258 

Branch pushed to git repo; I updated commit sha1. New commits:
1bb9ffa  TangentSpace, VectorBundleFiber: No construction

comment:41 Changed 4 months ago by
Commit:  1bb9ffa27ce20b7d17cd5808fb768ffcf8f18258 → 9c299cf7410b40b7e17da194433a0237be5bf9be 

Branch pushed to git repo; I updated commit sha1. New commits:
9c299cf  src/sage/categories/pushout.py: Expand doctest

comment:42 Changed 4 months ago by
Description:  modified (diff) 

comment:43 Changed 4 months ago by
Description:  modified (diff) 

comment:44 Changed 4 months ago by
Commit:  9c299cf7410b40b7e17da194433a0237be5bf9be → 2c6f7ee2dbc40bbdc599da9f4375d3533dd89b5f 

Branch pushed to git repo; I updated commit sha1. New commits:
2c6f7ee  src/sage/categories/pushout.py: Fix pycodestyle

comment:46 Changed 4 months ago by
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 longterm 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 classlevel doc to the new class explaining the difference.
comment:47 followup: 48 Changed 4 months ago by
Another hackish way would be to assign self.construction
in a __classcall_private__
method.
comment:48 Changed 4 months ago by
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 nonlocally (to the method).
comment:49 Changed 4 months ago by
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:51 Changed 3 months ago by
Commit:  2c6f7ee2dbc40bbdc599da9f4375d3533dd89b5f → a81428a82f09fa6abf9793151700f58ea903ced2 

Branch pushed to git repo; I updated commit sha1. New commits:
165e6ff  Revert "src/doc/en/thematic_tutorials/tutorialimplementingalgebraicstructures.rst: In subclassing example, import CombinatorialFreeModule"

c95e97f  Revert "Update doctest outputs for new class CombinatorialFreeModule_with_construction"

f22988f  Revert "sage.combinat.all: Import CombinatorialFreeModule_with_construction as CombinatorialFreeModule"

9a2c87e  Revert "CombinatorialFreeModule_with_construction: Add __classcall_private__"

d78a002  Revert "CombinatorialFreeModule: Move method 'contruction' to subclass CombinatorialFreeModule_with_construction"

a81428a  CombinatorialFreeModule.construction: On subclasses, return None

comment:52 Changed 3 months ago by
Commit:  a81428a82f09fa6abf9793151700f58ea903ced2 → 3f4417442c3a8e396e121b054794aa18b4d82154 

Branch pushed to git repo; I updated commit sha1. New commits:
3f44174  src/sage/tensor/modules/finite_rank_free_module.py, src/sage/categories/pushout.py: Fix name mappings

comment:53 followup: 54 Changed 3 months ago by
Cc:  Nicolas M. Thiéry added 

Reviewers:  → Travis Scrimshaw 
I think this is acceptable. Éric, do you have any comments?
comment:54 Changed 3 months ago by
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:57 Changed 3 months ago by
Branch:  u/mkoeppe/add_construction_methods_to_finiterankfreemodule_and_combinatorialfreemodule → 3f4417442c3a8e396e121b054794aa18b4d82154 

Resolution:  → fixed 
Status:  positive_review → closed 
New commits:
Create CombinatorialFreeModule if basis keys given
zero_vector: Handle bad input like zero_vector(3.6) before passing it to ZZ.__pow__
FreeModule: SImplify argument handling
src/sage/misc/sageinspect.py: Do not use FreeModule as an example for a class instance
Extend VectorSpace factory in the same way
Move documentation from FreeModuleFactory to FreeModule
FreeModule: Add to documentation
More documentation