MultiCoordFunction: Make it the element class of the free module over ChartFunctionRing

[mkoeppe]

Currently, MultiCoordFunction is not an element class.

sage: M = Manifold(2, 'M', structure='topological')
sage: X.<x,y> = M.chart()
sage: FR = X.function_ring()
sage: f = X.multifunction(x+y, sin(x*y), x^2 + 3*y); f
Coordinate functions (x + y, sin(x*y), x^2 + 3*y) on the Chart (M, (x, y))
sage: f.parent()
<class 'sage.manifolds.chart_func.MultiCoordFunction'>
sage: FR^3                                                 # with #31721
Ambient free module of rank 3 over Ring of chart functions on Chart (M, (x, y))
sage: f in FR^3
False

We change the function sage.modules.free_module.element_class so that it uses the element class Vector_symbolic_dense not only for free modules over the ring SR but also for free modules over any commutative ring whose base ring is SR.

In fact, we dispatch through a new method _free_module_element_class_dense. ChartFunction provides a specialized implementation, which makes sure that MultiCoordFunction (now a subclass of Vector_symbolic_dense) is used as the element class.

This simplifies the implementation of MultiCoordFunction and also makes the elementwise symbolic methods simplify etc. available.

Follow-up:

• We could move the method MultiCoordFunction.jacobian to the superclass, making it more generally available.

Depends on #31721
Depends on #31999

CC: @egourgoulhon @mjungmath @tscrim

Component: manifolds

Work Issues: rebase on #31999; fix pickling issue

Author: Matthias Koeppe

Branch/Commit: u/mkoeppe/multicoordfunction__make_it_the_element_class_of_the_free_module_over_chartfunctionring @ fb31802

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

[mkoeppe]

Branch: u/mkoeppe/multicoordfunction__make_it_the_element_class_of_the_free_module_over_chartfunctionring

[sagetrac-git]

Commit: dbfe7ac

[sagetrac-git]

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

dbfe7ac

src/sage/modules/free_module.py: Update copyright years according to 'git blame -w --date=format:%Y src/sage/modules/free_module.py | sort -k2'

[sagetrac-git]

Changed commit from dbfe7ac to cfd813b

[sagetrac-git]

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

cfd813b

sage.modules.free_module.element_class: For rings with SR base ring, delegate to new method _free_module_element_class_dense

[sagetrac-git]

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

278aa2b

CallableSymbolicExpressionRing_class: Fix doctest

[sagetrac-git]

Changed commit from cfd813b to 278aa2b

[sagetrac-git]

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

7bbb5f8

MultiCoordFunction: Make it an element class

[sagetrac-git]

Changed commit from 278aa2b to 7bbb5f8

[sagetrac-git]

Changed commit from 7bbb5f8 to 27e3889

[sagetrac-git]

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

fa5057c	Trac 31721: `__pow__` method for parents
27422d3	31721: more comments
e8d3427	clarify comment and doctest for multiple inheritance
b873bcb	some doc
27e3889	Merge #31721

[sagetrac-git]

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

3c36bd2

Eliminate use of MultiCoordFunction._functions

[sagetrac-git]

Changed commit from 27e3889 to 3c36bd2

[egourgoulhon]

comment:8

Looks nice. However, the pickling of continuous maps fails with the new code for MultiCoordFunction. More precisely, the equality test of the pickling fails when attempting to compare the MultiCoordFunction's describing the continuous maps is a given pair of charts, with the error

File "/home/eric/sage/9.4.develop/local/lib/python3.8/site-packages/sage/manifolds/continuous_map.py", 
line 577, in __eq__
...
AttributeError: 'sage.modules.free_module_element.FreeModuleElement_generic_dense'
object  has no attribute 'expr'

[mkoeppe]

comment:9

Thanks for taking a look! Because of these errors I hadn't set the ticket to "needs review" yet. I'll revise it in the next few days, but help is welcome.

[sagetrac-git]

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

49f63a4	Vector_symbolic_dense: Fix pickling
dcdd412	src/sage/manifolds/continuous_map.py: Update doctest output
fb31802	src/sage/manifolds/chart.py: Update doctest output

[sagetrac-git]

Changed commit from 3c36bd2 to fb31802

[mkoeppe]

Author: Matthias Koeppe

[mkoeppe]

comment:12

sage -t --long --random-seed=0 src/sage/algebras/lie_algebras/quotient.py  # 1 doctest failed
sage -t --long --random-seed=0 src/sage/modules/free_module_element.pyx  # 1 doctest failed

[mkoeppe]

comment:13

I think I could use some help from a pickling expert here...

[mjungmath]

comment:14

I bet this is related to #31182.

[mjungmath]

comment:15

Okay, maybe not.

[egourgoulhon]

comment:16

Replying to @mkoeppe:

I think I could use some help from a pickling expert here...

Sorry I am definitely not such an expert and therefore cannot offer help here. Instead, let me ask about the motivation of this ticket. I understand that, from a mathematical point of view, MultiCoordFunction's belong to the free module FR^n (FR being the ring of chart functions associated to a given chart and n the manifold's dimension) and that implementing this simplifies (a little) the code by relying on some code already implemented for Vector_symbolic_dense. This, by itself, is a motivation. However, I would say this is not a strong one, because no use is made in the manifold code of the module structure of the set of MultiCoordFunction's over a given chart. Indeed, in the current implementation, the class MultiCoordFunction is used only to store

1. the coordinate expression of a continous map in a given pair of charts;
2. the transition map between two charts.

In both cases, there is no meaning in the basic module operations m1 + m2 and f*m1, where m1 and m2 are two MultiCoordFunction's and f is a chart function. Hence, the motivation to provide a parent with a module structure seems pretty weak, given the current use of the class MultiCoordFunction. So maybe it is not worth to pursue the effort and keep MultiCoordFunction as a simple technical storage class (by opposition to a "mathematical" class), not relying on sophisticated parts of Sage, with complicated pickling and possibly less efficient in terms of CPU time.

[mkoeppe]

comment:17

My main motivation for the present ticket is for defining (not necessarily smooth) vector-valued (and later matrix-valued #31988) functions on manifolds (for #31981). If another class is better suited for this, I'd be interested to know.

Some fixes here on the ticket are needed to make free module elements over the ring of coordinate functions work properly. Then I realized that MultiCoordFunction could just be the element class, giving some mild simplifications of the code. I agree that it is not essential to change MultiCoordFunction -- but whether I use that or a new class as the element class, I will need to fix the pickling of the elements.

[mjungmath]

comment:18

Regarding the pickling, can you spot after which commit the error occurred; more precisely, which lines caused it?

[egourgoulhon]

comment:19

Replying to @mkoeppe:

My main motivation for the present ticket is for defining (not necessarily smooth) vector-valued (and later matrix-valued #31988) functions on manifolds (for #31981).

Thanks for your quick reply. This sounds a strong motivation for a module structure!

If another class is better suited for this, I'd be interested to know.

Wouldn't a kind of MultiScalarField be better suited? In some sense, for vector-valued functions, this already exists through the class Components.

Some fixes here on the ticket are needed to make free module elements over the ring of coordinate functions work properly. Then I realized that MultiCoordFunction could just be the element class, giving some mild simplifications of the code. I agree that it is not essential to change MultiCoordFunction -- but whether I use that or a new class as the element class, I will need to fix the pickling of the elements.

Thanks for these explanations.

[mkoeppe]

comment:20

Replying to @egourgoulhon:

Replying to @mkoeppe:

My main motivation for the present ticket is for defining (not necessarily smooth) vector-valued (and later matrix-valued #31988) functions on manifolds (for #31981).

Thanks for your quick reply. This sounds a strong motivation for a module structure!

If another class is better suited for this, I'd be interested to know.

Wouldn't a kind of MultiScalarField be better suited? In some sense, for vector-valued functions, this already exists through the class Components.

You are probably right about this. I guess a tricky point is regarding the required smoothness of scalar fields. I like to think of the domain as a smooth manifold, but the functions that I want to define on them often are less smooth - typically only piecewise C¹, with Lipschitz gradients. See for example https://www.cvxpy.org/api_reference/cvxpy.atoms.elementwise.html

[mkoeppe]

Changed dependencies from #31721 to #31721, #31999

[mkoeppe]

Work Issues: rebase on #31999; fix pickling issue

[mkoeppe]

comment:23

Setting a new milestone for this ticket based on a cursory review.