#19448 closed enhancement (fixed)
Improvements to submodules
Reported by:  Travis Scrimshaw  Owned by:  Travis Scrimshaw 

Priority:  major  Milestone:  sage9.2 
Component:  categories  Keywords:  sd109 
Cc:  John Palmieri, Nicolas M. Thiéry, Darij Grinberg, Aladin Virmaux, Frédéric Chapoton, Matthias Köppe  Merged in:  
Authors:  Travis Scrimshaw  Reviewers:  Simon Brandhorst 
Report Upstream:  N/A  Work issues:  
Branch:  d8a1c50 (Commits, GitHub, GitLab)  Commit:  
Dependencies:  Stopgaps: 
Description
We implement the lift map as a coercion map from a submodule and extend the submodule to work over modules with infinite bases.
Change History (15)
comment:1 Changed 7 years ago by
Branch:  → public/categories/improve_submodules19448 

Cc:  Darij Grinberg Aladin Virmaux added 
Commit:  → 98dc756161108b5d01280c54d7b475ab4dd59489 
Status:  new → needs_review 
comment:2 Changed 7 years ago by
Milestone:  sage6.10 → sage7.2 

comment:3 Changed 7 years ago by
Commit:  98dc756161108b5d01280c54d7b475ab4dd59489 → 8006b8eab80e772ab46bd440429703717869f5cf 

Branch pushed to git repo; I updated commit sha1. New commits:
8006b8e  Merge branch 'public/categories/improve_submodules19448' of trac.sagemath.org:sage into public/categories/improve_submodules19448

comment:5 Changed 5 years ago by
Commit:  8006b8eab80e772ab46bd440429703717869f5cf → d4838df190198c014f700c8dfea4062c7cf82a62 

comment:6 Changed 5 years ago by
Cc:  Frédéric Chapoton added 

Milestone:  sage7.2 → sage8.0 
Status:  needs_work → needs_review 
Actually, the more I think about it, the more I am for fixing the ordering of the submodule basis as the submodule is defined by a matrix, where there is an implicit ordering of the basis already in there.
comment:8 Changed 2 years ago by
Commit:  d4838df190198c014f700c8dfea4062c7cf82a62 → 1267467269fc838d5c385e13973b6791b1dc3d79 

comment:9 Changed 2 years ago by
Cc:  Matthias Köppe added 

Milestone:  sage8.0 → sage9.2 
Status:  needs_work → needs_review 
This should now pass all tests. I also extended the echelon_form
to handle the case when the ambient module is infinite dimensional.
comment:10 Changed 2 years ago by
Keywords:  sd109 added 

comment:11 Changed 2 years ago by
Commit:  1267467269fc838d5c385e13973b6791b1dc3d79 → d8a1c50f2044b42edd8f136bc89168a1eb858b6c 

Branch pushed to git repo; I updated commit sha1. New commits:
d8a1c50  Fixing doctests and making _vector_ and from_vector methods more consistent.

comment:12 Changed 2 years ago by
So doctests were failing elsewhere because _support_order
was also an attribute used by the submodules. So I refactored the method to now be called _compute_support_order
. I also found some other issues with to/from_vector
not being consistent in their parameters. Doctests that were failing now pass.
comment:13 Changed 2 years ago by
Reviewers:  → Simon Brandhorst 

Status:  needs_review → positive_review 
looks reasonable to me.
comment:14 Changed 2 years ago by
Branch:  public/categories/improve_submodules19448 → d8a1c50f2044b42edd8f136bc89168a1eb858b6c 

Resolution:  → fixed 
Status:  positive_review → closed 
comment:15 Changed 2 years ago by
Commit:  d8a1c50f2044b42edd8f136bc89168a1eb858b6c 

Followup discussion in https://groups.google.com/g/sagedevel/c/z1k8IT3ocR4/m/UfkhlDweBwAJ
One thing that might be slightly controversial is that I construct an explicit ordering of elements used after the echelonization. However, this is needed to be stored in the submodule because if the ordering of the ambient changes (which it can by the
set_order
method), this could break the triangularity of thelift
map. This also allowed me to construct submodules of infinitedimensional modules.New commits:
Improvements to submodules.