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

Refine category of ScalarField #31883

Closed
mkoeppe opened this issue May 31, 2021 · 16 comments
Closed

Refine category of ScalarField #31883

mkoeppe opened this issue May 31, 2021 · 16 comments

Comments

@mkoeppe
Copy link
Member

mkoeppe commented May 31, 2021

A ScalarField on a topological manifold is supposed to be a continuous map -- but this is not reflected by the category of its parent.

sage: M = Manifold(2, 'R^2', structure='topological')
sage: c_cart.<x,y> = M.chart()
sage: r_squared = M.scalar_field(x^2+y^2)
sage: r_squared.parent().category()
Category of commutative algebras over Symbolic Ring
sage: r_squared.codomain()
AttributeError: 'ScalarFieldAlgebra_with_category.element_class' object has no attribute 'codomain'

CC: @egourgoulhon @tscrim @mjungmath

Component: manifolds

Author: Matthias Koeppe

Branch/Commit: cd3ca79

Reviewer: Travis Scrimshaw

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

@mkoeppe mkoeppe added this to the sage-9.4 milestone May 31, 2021
@mkoeppe
Copy link
Member Author

mkoeppe commented May 31, 2021

comment:1

Unfortunately, trying to make ScalarField a subclass of Map leads to:

---> 50 class ScalarField(CommutativeAlgebraElement, ModuleElementWithMutability, Map):
     51     r"""
     52     Scalar field on a topological manifold.

TypeError: multiple bases have instance lay-out conflict

@mkoeppe
Copy link
Member Author

mkoeppe commented May 31, 2021

@mkoeppe
Copy link
Member Author

mkoeppe commented May 31, 2021

Author: Matthias Koeppe

@mkoeppe
Copy link
Member Author

mkoeppe commented May 31, 2021

Commit: 1942c71

@mkoeppe
Copy link
Member Author

mkoeppe commented May 31, 2021

comment:3

Anyway, here is a simple solution


New commits:

1942c71ScalarField.codomain: New, put scalar fields in category of continuous maps

@tscrim
Copy link
Collaborator

tscrim commented May 31, 2021

comment:4

Green bot => positive review.

@tscrim
Copy link
Collaborator

tscrim commented May 31, 2021

Reviewer: Travis Scrimshaw

@tscrim
Copy link
Collaborator

tscrim commented Jun 1, 2021

comment:5
sage -t --long --random-seed=0 src/sage/graphs/graph.py  # 1 doctest failed
sage -t --long --random-seed=0 src/sage/manifolds/differentiable/scalarfield_algebra.py  # 2 doctests failed
sage -t --long --random-seed=0 src/doc/en/thematic_tutorials/vector_calculus/vector_calc_advanced.rst  # 1 doctest failed
sage -t --long --random-seed=0 src/doc/en/thematic_tutorials/vector_calculus/vector_calc_plane.rst  # 1 doctest failed

The one in graph.py doesn't seem like an error related to this ticket, but the others are trivial. Please make the changes and check that graph.py is indeed unrelated. Once done, you can set a positive review.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 1, 2021

Changed commit from 1942c71 to cd3ca79

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 1, 2021

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

cd3ca79Update doctests for refined category of ScalarField

@mkoeppe
Copy link
Member Author

mkoeppe commented Jun 1, 2021

comment:7

The failing doctest in graph.py is unrelated and is fixed by #31848

@tscrim
Copy link
Collaborator

tscrim commented Jun 1, 2021

comment:8

Thanks.

@mkoeppe
Copy link
Member Author

mkoeppe commented Jun 1, 2021

comment:9

Thank you!

@egourgoulhon
Copy link
Member

comment:10

I agree with the code added in this ticket, but let me seize the opportunity to point out some current inconsistency in the manifold set up, which is due to the lack of a proper real field in Sage (cf. #24456):

  1. the base field of a real manifold is RR
  2. the base ring of the algebra of scalar fields on a manifold is SR

Of course, from a pure mathematical side, 1 and 2 should be identical.

The basic reason for 1 is RR being the simplest proxy for the genuine real field, albeit all (symbolic) computations on manifolds have nothing to do with 53 bits of precision...

The basic reason for 2 is to naturally allow for algebra operations like a*f, where a is a symbolic variable and f a scalar field.

If #24456 converges some day, both 1 and 2 should be set to the real field object that will emerge from it. Meanwhile, we have

sage: M = Manifold(2, 'M')
sage: CM = M.scalar_field_algebra()
sage: M.base_field()
Real Field with 53 bits of precision
sage: CM.base_ring()
Symbolic Ring

With the codomain() method introduced in this ticket, we have in addition

sage: X.<x,y> = M.chart() 
sage: f = M.scalar_field(x^2)
sage: f.codomain() is f.domain().base_field()
True

which is nice, but

sage: f.codomain() is f.parent().base_ring()
False

Moreover, for images of scalar fields, we have

sage: p = M((pi, 0))
sage: f(p)
pi^2
sage: f(p) in f.codomain()
True

but this holds only because pi^2 can be converted to an element of RR. Otherwise, the test fails:

sage: a = var('a', domain='real')
sage: p = M((a, 0))
sage: f(p) in f.codomain()
False

f(p) not lying in f.codomain() is certainly something we do not want, but this is currently a consequence of the inconsistency 1 <-> 2 mentioned above. The latter could be removed by making SR the base field of manifolds. But then a real manifold and a complex one would appear as having the same base field (according to base_field()).

Finally, it's worth to stress that in practice, the inconsistency 1 <-> 2 is a rather mild one: to my knowledge, it has never triggered any error nor yield any false result in actual computations on manifolds (apart from the trivial CM.base_ring() is M.base_field() being false).

@mkoeppe
Copy link
Member Author

mkoeppe commented Jun 2, 2021

comment:11

Thanks a lot for sharing these insights!

@vbraun
Copy link
Member

vbraun commented Jun 29, 2021

Changed branch from u/mkoeppe/refine_category_of_scalarfield to cd3ca79

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

4 participants