Opened 9 months ago

Closed 3 months ago

#33431 closed enhancement (fixed)

Category of submodules of sage.modules.free_module.FreeModule_ambient_pid

Reported by: Matthias Köppe Owned by:
Priority: major Milestone: sage-9.8
Component: linear algebra Keywords:
Cc: Travis Scrimshaw, gh-louisng114 Merged in:
Authors: Matthias Koeppe Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: 62f28a3 (Commits, GitHub, GitLab) Commit: 62f28a334262b6b6712c0a6d98e07291abcba5e2
Dependencies: Stopgaps:

Status badges

Description (last modified by Matthias Köppe)

sage: (QQ^2).submodule([[1,1]])
Vector space of degree 2 and dimension 1 over Rational Field
Basis matrix:
[1 1]
sage: _.category()
Category of finite dimensional vector spaces with basis over (number fields and quotient fields and metric spaces)

Subobjects is missing, so the standard methods ambient, lift, retract are missing.

This is in contrast to CombinatorialFreeModule.

(There is a class-specific method ambient_module though.)

Also there is an apparent conflict of the element method lift with the one defined in free_module_element.pyx; not resolved here.

Change History (19)

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

Milestone: sage-9.6sage-9.7

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

Milestone: sage-9.7sage-9.8

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

Branch: u/mkoeppe/category_of_submodules_of_sage_modules_free_module_freemodule_ambient_pid

comment:4 Changed 3 months ago by git

Commit: 88c67d9704e6100b9585336802e2a746e7c15104

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

88c67d9FreeModule_submodule_with_basis_pid.{lift,retract}: New

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

Authors: Matthias Koeppe
Status: newneeds_review

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

Description: modified (diff)

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

Description: modified (diff)

comment:8 Changed 3 months ago by git

Commit: 88c67d9704e6100b9585336802e2a746e7c15104823073652e1a1e9f734589e6d5c74d0744de8f32

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

edec50esrc/sage/modules/free_module.py, src/sage/modules/fg_pid/fgp_morphism.py: Use ModulesWithBasis instead of FreeModules, which is only an alias defined in sage.categories.all
8230736FreeModule_submodule_with_basis_pid: Fix up category

comment:9 Changed 3 months ago by Travis Scrimshaw

LGTM other than catching Exception seems quite broad and might hide bugs. Could we make that more refined?

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

I generally agree (and specifically it did hide a bug, just fixed in 8230736!) but this is copied directly from Module_free_ambient.__init__ and I don't have special insights to what it should be tightened.

comment:11 Changed 3 months ago by Travis Scrimshaw

I would do the standard ones: ValueError, TypeError, AttributeError, and maybe NotImplementedError. If it is raising something outside of these (such as an assert) then it almost certainly should be another issue to address.

comment:12 Changed 3 months ago by git

Commit: 823073652e1a1e9f734589e6d5c74d0744de8f322929a9e799ab5a913f753bc4e46b78aaae4180be

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

2929a9esrc/sage/modules/free_module.py: Make 'except Exception' more specific

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

OK. I've made this change (in both places)

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

+        if category is None:
+            from sage.categories.modules_with_basis import ModulesWithBasis
+            category = ModulesWithBasis(R.category()).FiniteDimensional()
+            try:
+                if R.is_finite() or len(basis) == 0:
+                    category = category.Enumerated().Finite()
+            except (ValueError, TypeError, AttributeError, NotImplementedError):
+                pass
+        category = category.Subobjects()

I guess I should probably change this to category &= ModulesWithBasis(R.category()).Subobjects()?

comment:15 Changed 3 months ago by Travis Scrimshaw

That would probably be better since it is not promising to be, e.g., a subalgebra.

comment:16 Changed 3 months ago by git

Commit: 2929a9e799ab5a913f753bc4e46b78aaae4180be62f28a334262b6b6712c0a6d98e07291abcba5e2

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

62f28a3src/sage/modules/free_module.py: Compute category more carefully

comment:17 Changed 3 months ago by Travis Scrimshaw

Reviewers: Travis Scrimshaw
Status: needs_reviewpositive_review

Thanks. LGTM.

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

Thank you!

comment:19 Changed 3 months ago by Volker Braun

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