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

Improvements to submodules #19448

Closed
tscrim opened this issue Oct 21, 2015 · 22 comments
Closed

Improvements to submodules #19448

tscrim opened this issue Oct 21, 2015 · 22 comments

Comments

@tscrim
Copy link
Collaborator

tscrim commented Oct 21, 2015

We implement the lift map as a coercion map from a submodule and extend the submodule to work over modules with infinite bases.

CC: @jhpalmieri @nthiery @darijgr @avirmaux @fchapoton @mkoeppe

Component: categories

Keywords: sd109

Author: Travis Scrimshaw

Branch: d8a1c50

Reviewer: Simon Brandhorst

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

@tscrim tscrim added this to the sage-6.10 milestone Oct 21, 2015
@tscrim tscrim self-assigned this Oct 21, 2015
@tscrim
Copy link
Collaborator Author

tscrim commented Oct 21, 2015

@tscrim
Copy link
Collaborator Author

tscrim commented Oct 21, 2015

comment:1

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 the lift map. This also allowed me to construct submodules of infinite-dimensional modules.


New commits:

98dc756Improvements to submodules.

@tscrim
Copy link
Collaborator Author

tscrim commented Oct 21, 2015

Commit: 98dc756

@tscrim tscrim modified the milestones: sage-6.10, sage-7.2 Mar 10, 2016
@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 29, 2016

Changed commit from 98dc756 to 8006b8e

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 29, 2016

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

8006b8eMerge branch 'public/categories/improve_submodules-19448' of trac.sagemath.org:sage into public/categories/improve_submodules-19448

@cheuberg
Copy link
Contributor

comment:4

merge conflict

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 9, 2017

Changed commit from 8006b8e to d4838df

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 9, 2017

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

147583bMerge branch 'public/categories/improve_submodules-19448' of git://trac.sagemath.org/sage into public/categories/improve_submodules-19448
d4838dfRemoving cmp for key.

@tscrim
Copy link
Collaborator Author

tscrim commented Jun 9, 2017

comment:6

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.

@tscrim tscrim modified the milestones: sage-7.2, sage-8.0 Jun 9, 2017
@simonbrandhorst
Copy link

comment:7

failing doctest

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 29, 2020

Changed commit from d4838df to 1267467

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 29, 2020

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

09f8caaMerge branch 'public/categories/improve_submodules-19448' of git://trac.sagemath.org/sage into public/categories/improve_submodules-19448
1267467Fixing failures and allowing echelon_form to handle oo-dim modules.

@tscrim
Copy link
Collaborator Author

tscrim commented May 29, 2020

comment:9

This should now pass all tests. I also extended the echelon_form to handle the case when the ambient module is infinite dimensional.

@tscrim tscrim removed this from the sage-8.0 milestone May 29, 2020
@tscrim tscrim added this to the sage-9.2 milestone May 29, 2020
@mkoeppe
Copy link
Contributor

mkoeppe commented May 29, 2020

Changed keywords from none to sd109

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 2, 2020

Changed commit from 1267467 to d8a1c50

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 2, 2020

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

d8a1c50Fixing doctests and making `_vector_` and from_vector methods more consistent.

@tscrim
Copy link
Collaborator Author

tscrim commented Jun 2, 2020

comment:12

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.

@simonbrandhorst
Copy link

comment:13

looks reasonable to me.

@simonbrandhorst
Copy link

Reviewer: Simon Brandhorst

@vbraun
Copy link
Member

vbraun commented Jun 21, 2020

Changed branch from public/categories/improve_submodules-19448 to d8a1c50

@mkoeppe
Copy link
Contributor

mkoeppe commented Nov 24, 2020

comment:15

Follow-up discussion in https://groups.google.com/g/sage-devel/c/z1k8IT3ocR4/m/UfkhlDweBwAJ

@mkoeppe
Copy link
Contributor

mkoeppe commented Nov 24, 2020

Changed commit from d8a1c50 to none

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

5 participants