free_module method for finite fields, number fields and p-adics

[roed314]

In support of the ring extension classes in #21413, we want to generalize the functionality of the vector_space method to free modules over other rings. This ticket renames the method to free_module and implements it for p-adic extensions.

CC: @saraedum @xcaruso

Component: algebra

Keywords: padicBordeaux

Author: David Roe

Branch/Commit: 242fecc

Reviewer: Xavier Caruso

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

[roed314]

Branch: u/roed/free_module

[roed314]

Author: David Roe

[roed314]

Changed keywords from none to padicBordeaux

[roed314]

Commit: 23947e9

[roed314]

New commits:

7d88f9a	Working on p-adic free module isomorphisms
f4f17d6	Fix precision problem in vector space maps by using new method _polynomial_list
23947e9	Working on vector_space and free_module

[tscrim]

comment:4

The free_module in padic_extension_generic.py does not have any doctests.

About this comment

# We could have used morphisms in the category
# FiniteDimensionalModulesWithBasis over Qp(p)
# But this would require making p-adic fields
# part of this category which has a lot of side
# effects (adding methods which will show up
# in tab completion for example).  For now we
# just stick with Map.

having extra methods via tab completion is a good thing, especially as they are all still applicable to the objects. That specific reason really is not a good argument for not doing that, but are there other side effects that cause problems?

-            - ``basis`` -- a basis of the field as a vector space
-              over the subfield.  If not given, one is chosen automatically.
+            - ``basis`` -- a basis of the field as a vector space
+              over the subfield; f not given, one is chosen automatically
 
-            - ``map`` -- whether to return maps from and to the vector space.
+            - ``map`` -- whether to return maps from and to the vector space

-        - ``basis`` -- a basis for the vector space (ignored)
+        - ``basis`` -- (ignored) a basis for the vector space
 
-        - ``map`` -- whether to return maps to and from the vector space (default True)
+        - ``map`` -- (default: ``True``) whether to return maps to and from the vector space

-        - ``base`` -- a subfield (default: ``None``), the returned vector
+        - ``base`` -- a subfield (default: ``None``); the returned vector
           space is over this subfield `R`, which defaults to the base field of this
-          function field.
+          function field
 
-        - ``basis`` -- a basis for this field over the base.
 
         - ``maps`` -- boolean (default ``True``), whether to return
-          `R`-linear maps to and from `V`.
+          `R`-linear maps to and from `V`

-        - ``basis`` -- a list of elements giving a basis over the subfield (optional)
+        - ``basis`` -- (optional) a list of elements giving a basis over the subfield
 
+        - ``map`` -- whether to return isomorphisms to and from the vector space (default ``True``)
+        - ``map`` -- (default: ``True``) whether to return isomorphisms to and from the vector space

Also Returns -> Return.

[sagetrac-git]

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

8db48a2

Reviewer suggestions

[sagetrac-git]

Changed commit from 23947e9 to 8db48a2

[sagetrac-git]

Changed commit from 8db48a2 to 79f2458

[sagetrac-git]

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

79f2458

One more Returns

[roed314]

comment:7

Replying to @tscrim:

The free_module in padic_extension_generic.py does not have any doctests.

Thanks; fixed.

About this comment

# We could have used morphisms in the category
# FiniteDimensionalModulesWithBasis over Qp(p)
# But this would require making p-adic fields
# part of this category which has a lot of side
# effects (adding methods which will show up
# in tab completion for example).  For now we
# just stick with Map.

having extra methods via tab completion is a good thing, especially as they are all still applicable to the objects. That specific reason really is not a good argument for not doing that, but are there other side effects that cause problems?

Yep.

• If I add ModulesWithBasis(Qp(5)).Finite() as a base ring for Qq(125) I get errors on object creation. It's not clear to me how much work it would be to address all the issues introduced by this change, and I'm not interested in doing so at the moment when I have higher priority things to work on.
• The dimension() method of the ModulesWithBasis category doesn't take a base field. Two-step extensions are vector spaces both over their base field and over Qp; I don't think it would be possible to have both. But A class for ring extensions #21413 is exactly about creating relative extensions where we need to work with different base fields.

[tscrim]

comment:8

Replying to @roed314:

Replying to @tscrim:

About this comment

# We could have used morphisms in the category
# FiniteDimensionalModulesWithBasis over Qp(p)
# But this would require making p-adic fields
# part of this category which has a lot of side
# effects (adding methods which will show up
# in tab completion for example).  For now we
# just stick with Map.

having extra methods via tab completion is a good thing, especially as they are all still applicable to the objects. That specific reason really is not a good argument for not doing that, but are there other side effects that cause problems?

Yep.

Thanks for the explanations. Do you think you could change the above comment to state one of them you gave? Once done, then positive review.

• If I add ModulesWithBasis(Qp(5)).Finite() as a base ring for Qq(125) I get errors on object creation. It's not clear to me how much work it would be to address all the issues introduced by this change, and I'm not interested in doing so at the moment when I have higher priority things to work on.

No problem. It is something beyond the current ticket and not important for your applications. It is something I could look into too when I have more time as well.

• The dimension() method of the ModulesWithBasis category doesn't take a base field. Two-step extensions are vector spaces both over their base field and over Qp; I don't think it would be possible to have both. But A class for ring extensions #21413 is exactly about creating relative extensions where we need to work with different base fields.

In the mathematical sense, the category stores this information (i.e., it is an R-module, so it returns the dimension as an R-module), although we have partially removed this information. However, that is a bit tangential to this issue. I don't know the code, but your implementation of dimension should be overriding that. If instead it something coming from the two categories, then you will unfortunately just need to make it explicit somewhere. Or we just add that as an optional argument to the ModulesWithBasis category that is generally ignored as it is something that makes sense for all objects in that category. However, this should be done on a later ticket.

[sagetrac-git]

Changed commit from 79f2458 to 9313b2c

[sagetrac-git]

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

9313b2c

Adjust comment

[xcaruso]

comment:10

I'm not sure that checking equality with rich_to_bool(op,0) will always work.

In the doctest of the _call_ method in the class MapOneStepToFreeModule, the variable a is used two times with a different meaning.

[sagetrac-git]

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

ad98d9c

Address reviewer comments

[sagetrac-git]

Changed commit from 9313b2c to ad98d9c

[roed314]

comment:12

Thanks; fixed.

[xcaruso]

comment:13

LGTM if patch bot is happy

[xcaruso]

Reviewer: Xavier Caruso

[xcaruso]

comment:14

Maybe, for convenience:

• free_module should be implemented when base is self (in which case V = K and maps in both directions are the identity)
• free_module should be implemented when base is self.base_ring() (and not just None); this is currently not the case for number fields and p-adic fields
• free_module should be implemented when self is Qp or QQ

[xcaruso]

comment:16

By the way, the patchbot reported small issues (pyflakes + one method not documented)

[sagetrac-git]

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

507ab36

fix pyflakes

[sagetrac-git]

Changed commit from ad98d9c to 507ab36

[sagetrac-git]

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

a750d40

Fix doctest failure in Judson's book

[sagetrac-git]

Changed commit from 507ab36 to a750d40

[roed314]

comment:19

I think I fixed the doctest and pyflakes errors. Setting to Needs Review so that the patchbot can check. I haven't addressed Xavier's most recent comments yet....

[sagetrac-git]

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

b4147b0

Add missing backtick

[sagetrac-git]

Changed commit from a750d40 to b4147b0

[sagetrac-git]

Changed commit from b4147b0 to 242fecc

[sagetrac-git]

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

242fecc

Add free_module method to Rings()

[xcaruso]

comment:22

Thanks

[fchapoton]

comment:23

moving milestone to 9.0 (after release of 8.9)

[vbraun]

Changed branch from u/roed/free_module to 242fecc