Opened 6 years ago
Closed 5 years ago
#18843 closed enhancement (fixed)
Differentiable manifolds: vector fields and tensor fields
Reported by:  egourgoulhon  Owned by:  egourgoulhon 

Priority:  major  Milestone:  sage7.4 
Component:  geometry  Keywords:  differentiable manifold, tensor field, vector field, differential form 
Cc:  mbejger, bpillet, bpage  Merged in:  
Authors:  Eric Gourgoulhon, Michal Bejger  Reviewers:  Travis Scrimshaw 
Report Upstream:  N/A  Work issues:  
Branch:  fb7f4dd (Commits, GitHub, GitLab)  Commit:  fb7f4dd6e483f9956f49475bc16707f739f1129a 
Dependencies:  #15916, #18783, #20770  Stopgaps: 
Description (last modified by )
This ticket implements tensor fields (among which vector fields and differential forms) on differentiable manifolds. This is a followup of #18783 within the SageManifolds project (see the metaticket #18528 for an overview). As in #18783, the nondiscrete topological field K over which the differentiable manifold is defined is generic, although in most applications, K=R or K=C.
This ticket implements the following Python classes:
1/ Parent classes:
VectorFieldModule
: module of vector fields on a differentiable manifoldVectorFieldFreeModule
: free module of vector fields on a parallelizable differentiable manifoldTensorFieldModule
: module of tensor fields of a given type (k,l) on a differentiable manifoldTensorFieldFreeModule
: free module of tensor fields of a given type (k,l) on a parallelizable differentiable manifoldDiffFormModule
: module of differential forms of a given degree p (pforms) on a differentiable manifoldDiffFormFreeModule
: free module of differential forms of a given degree p (pforms) on a parallelizable differentiable manifoldAutomorphismFieldGroup
: general linear group of the module of vector fields on a differentiable manifoldAutomorphismFieldParalGroup
: general linear group of the free module of vector fields on a parallelizable differentiable manifold
2/ Element classes:
TensorField
: tensor field on a differentiable manifoldVectorField
: vector field on a differentiable manifoldDiffForm
: pform on differentiable manifoldAutomorphismField
: field of tangentspace automorphisms on a differentiable manifold
TensorFieldParal
: tensor field on a parallelizable differentiable manifoldVectorFieldParal
: vector field on a parallelizable differentiable manifoldDiffFormParal
: pform on parallelizable differentiable manifoldAutomorphismFieldParal
: field of tangentspace automorphisms on a parallelizable differentiable manifold
3/ Other classes:
VectorFrame
: vector frame on a differentiable manifoldCoordFrame
: coordinate vector frame on a differentiable manifold
CoFrame
: coframe (frame of 1forms) on a differentiable manifoldCoordCoFrame
: coordinate coframe on a differentiable manifold
Documentation:
The reference manual is produced by
sage docbuild reference/manifolds html
It can also be accessed online at http://sagemanifolds.obspm.fr/doc/18843/reference/manifolds/
More documentation (e.g. example worksheets) can be found here.
Change History (69)
comment:1 Changed 6 years ago by
 Commit changed from d48210c11f38f90ce656f0fa25ec550d147a1892 to b9c3bc529cdd9e7c11f5f6bd10f6e29b070b7cbc
comment:2 Changed 6 years ago by
 Commit changed from b9c3bc529cdd9e7c11f5f6bd10f6e29b070b7cbc to a2e0696fc335aa07d0b2ce3f6ab54c45996e6746
Branch pushed to git repo; I updated commit sha1. New commits:
a2e0696  Add method display() to tensor components and display_comp() to tensors

comment:3 Changed 6 years ago by
 Commit changed from a2e0696fc335aa07d0b2ce3f6ab54c45996e6746 to cad56b1f9b3f4ad59b957d7e8b1b8512bec3ae4d
Branch pushed to git repo; I updated commit sha1. New commits:
cad56b1  Improve documentation of tensor fields

comment:4 Changed 6 years ago by
 Commit changed from cad56b1f9b3f4ad59b957d7e8b1b8512bec3ae4d to 794c149286b90285cd1d32cd21145b2a2e3e8d53
Branch pushed to git repo; I updated commit sha1. New commits:
794c149  Improve documentation of differential forms

comment:5 Changed 6 years ago by
 Commit changed from 794c149286b90285cd1d32cd21145b2a2e3e8d53 to 435fb11339a9e00b87de5ceac36ed14085c197e0
Branch pushed to git repo; I updated commit sha1. New commits:
435fb11  Improve documentation of vector frames

comment:6 Changed 6 years ago by
 Commit changed from 435fb11339a9e00b87de5ceac36ed14085c197e0 to 8f47f64e2eb6411bcc00693f012c63974b3d4e28
Branch pushed to git repo; I updated commit sha1. New commits:
8f47f64  More improvements in the documentation of diff. manifolds

comment:7 Changed 6 years ago by
 Commit changed from 8f47f64e2eb6411bcc00693f012c63974b3d4e28 to 935a0f741185e30f50ad27750987d7bac63b957f
Branch pushed to git repo; I updated commit sha1. New commits:
935a0f7  Improvements in the documentation of class DiffManifold

comment:8 Changed 6 years ago by
 Description modified (diff)
comment:9 Changed 6 years ago by
 Commit changed from 935a0f741185e30f50ad27750987d7bac63b957f to 2c6b711ea7c8faedef4c13804c8e8f9466576c1a
Branch pushed to git repo; I updated commit sha1. New commits:
2c6b711  Improve documentation of scalar and vector fields; add doctests

comment:10 Changed 6 years ago by
 Commit changed from 2c6b711ea7c8faedef4c13804c8e8f9466576c1a to eeff027998d9e9c3dfec7de22543dd5501d4f768
Branch pushed to git repo; I updated commit sha1. New commits:
eeff027  Add doctests for tensor fields

comment:11 Changed 6 years ago by
 Commit changed from eeff027998d9e9c3dfec7de22543dd5501d4f768 to 686cee1a746cfe3e6aecf5e4087d492c11be8839
Branch pushed to git repo; I updated commit sha1. New commits:
686cee1  Full doctest coverage.

comment:12 Changed 6 years ago by
 Commit changed from 686cee1a746cfe3e6aecf5e4087d492c11be8839 to fcf8ec4d07fc26e065c47d83b8187ca89c6dae5f
Branch pushed to git repo; I updated commit sha1. New commits:
fcf8ec4  Improvement in simplify_sqrt_real() and remove of verbose=True in TestSuite().run()

comment:13 Changed 6 years ago by
 Dependencies changed from #15916, #18100, #18783 to #15916, #18783
comment:14 Changed 6 years ago by
 Milestone changed from sage6.8 to sage6.9
comment:15 Changed 6 years ago by
 Commit changed from fcf8ec4d07fc26e065c47d83b8187ca89c6dae5f to 307e655897455860869ede907a4619f11e024ecd
Branch pushed to git repo; I updated commit sha1. New commits:
307e655  Major improvements in the documentation of diff. manifolds (tensor field part)

comment:16 Changed 6 years ago by
 Commit changed from 307e655897455860869ede907a4619f11e024ecd to dc7f7a1405bae39afe1e582f440373e0a54fb78c
Branch pushed to git repo; I updated commit sha1. New commits:
dc7f7a1  Improve TensorField.__eq__ (case with no open cover known)

comment:17 Changed 6 years ago by
 Commit changed from dc7f7a1405bae39afe1e582f440373e0a54fb78c to 4d6f21c8ab3f399e97806baa18ec7d2dac714952
Branch pushed to git repo; I updated commit sha1. Last 10 new commits:
57b21df  Add doctest to set_axes_labels

e558c06  Improvement in simplify_sqrt_real(); TestSuite run nonverbose

d65f654  Introduce open covers on top manifolds + many improvements in the documentation

00c327d  Slight reorganization of the reference manual of topological manifolds (morphisms part)

f8d3f27  Merge #18725 into #18640

f2fef7b  Small improvements in the documentation of differentiable manifolds

8ab80d8  Improvement in simplify_sqrt_real(); minor modif. in documentation

2f231b6  Major improvements in the documentation of diff. manifolds (basic part)

f0ca2de  Merge #18783 into #18725

4d6f21c  Merge #18843 into #18783

comment:18 Changed 6 years ago by
 Description modified (diff)
 Milestone changed from sage6.9 to sage6.10
 Status changed from new to needs_review
comment:19 Changed 6 years ago by
 Commit changed from 4d6f21c8ab3f399e97806baa18ec7d2dac714952 to a527726fb1d474803c95309b70f3a1e160a1faee
Branch pushed to git repo; I updated commit sha1. Last 10 new commits:
f8f5b93  Fixing last remaining doctests.

041a5d1  Adding padics to metric spaces and some cleanup.

bfa0cdf  One last doc tweak.

d13c368  Fixing doc of metric spaces.

2605c0b  Merge #18529 (Topological manifolds: basics) into #18175 (Implement categories for topological...)

6dec6d5  Implement topological manifolds (basics, #18529) on the new categories for manifolds (#18175)

3403978  Implement top. manifolds (scalar fields, #18640) on the new manifold categories (#18175)

b0521ef  Implement top. manifolds (morphisms, #18725) on the new manifold categories (#18175)

f643097  Implement diff. manifolds (basics, #18783) on the new manifold categories (#18175)

a527726  Implement diff. manifolds (tensor fields, #18843) on the new manifold categories (#18175)

comment:20 Changed 6 years ago by
 Commit changed from a527726fb1d474803c95309b70f3a1e160a1faee to 0ee4c41ddf9b0759d219a7d256a91a0b3e6357c9
Branch pushed to git repo; I updated commit sha1. Last 10 new commits:
252e616  Remove UniqueRepresentation, leaving only WithEqualityById, for topological manifolds and charts.

e7139ab  Merge branch top_manif_basics without UniqueRepresentation into top_manif_scalar_fields.

e2f192f  Remove UniqueRepresentation, leaving only WithEqualityById, for scalar fields on topological manifolds

668bc26  Remove UniqueRepresentation, leaving only WithEqualityById, for topological manifold homsets

6518699  Introduce the attribute _field_type in class TopologicalManifold to check for real and complex manifolds.

22383e6  Check for real/complex manifold performed on base_field_type() instead of RR/CC

66f2c5a  Change function('f', x) to function('f')(x) to accomodate the deprecation warning introduced in #17447

a28ed04  Morphisms of topological manifolds with the use of base_field_type()

f31bed1  Remove UniqueRepresentation from differentiable manifolds

0ee4c41  Tensor fields on differentiable manifolds without unique representation

comment:21 Changed 6 years ago by
 Commit changed from 0ee4c41ddf9b0759d219a7d256a91a0b3e6357c9 to e8f11ffc706221d3f5a3b0c84d262519b1fa81e3
Branch pushed to git repo; I updated commit sha1. New commits:
e8f11ff  Fix pickling test in tensor field modules.

comment:22 Changed 6 years ago by
 Commit changed from e8f11ffc706221d3f5a3b0c84d262519b1fa81e3 to a9677872c7499a8c38044dbe8442f0821380c284
Branch pushed to git repo; I updated commit sha1. New commits:
a967787  Suppress direct call to _element_constructor_ in tensor field parent classes

comment:23 Changed 6 years ago by
 Commit changed from a9677872c7499a8c38044dbe8442f0821380c284 to 45beae5c3809cde39d51e72016138964557ac91b
Branch pushed to git repo; I updated commit sha1. New commits:
d8397c1  Merge branch 'public/manifolds/top_manif_basics' of trac.sagemath.org:sage into public/manifolds/top_manif_basics

0b08b11  Some small tweaks as part of the review.

d3e5d4d  Revert to UniqueRepresentation for topological manifolds

2481359  Revert to UniqueRepresentation for ScalarFieldAlgebra; better ScalarField constructor

0cc06e8  Revert to UniqueRepresentation for topological manifold homsets

322d5bd  Revert to UniqueRepresentation for differentiable manifolds

45beae5  Revert to UniqueRepresentation for differentiable manifolds: tensor fields

comment:24 Changed 6 years ago by
 Commit changed from 45beae5c3809cde39d51e72016138964557ac91b to 0d68f86562cdd80f239c3093ee32d16939e6cae2
Branch pushed to git repo; I updated commit sha1. New commits:
85d03dc  Change the argument type to structure in Manifold

5251ef0  Remove method _test_pickling from class TopologicalManifoldPoint

f69c9ee  Fix doctest error in coord_func_symb.py due to #19312 (update to pynac0.5.2)

7889a5d  Change in simplify_sqrt_real to cope with the change of != operator induced by #19312 (Sage 6.10.beta7)

e8d2ba6  Differentiable manifolds: basics with the change in symbolic expression logic induced by #19312 (Sage 6.10.beta7)

0d68f86  Diff. manifolds: tensor fields, with the change in symbolic expression logic induced by #19312 (Sage 6.10.beta7)

comment:25 Changed 6 years ago by
 Commit changed from 0d68f86562cdd80f239c3093ee32d16939e6cae2 to ecfbf2cd37624907f4d009d3096abd2b9c2312a9
Branch pushed to git repo; I updated commit sha1. Last 10 new commits:
cdfa817  Merge branch u/tscrim/top_manifolds_refactor into Sage 6.10.rc1 + public/manifolds/top_manif_basics

c5f35af  Make AbstractSet inherit from UniqueRepresentation; correct doctests; start to change documentation.

3a52500  Some class renaming; add more examples and doctests (full coverage)

a1ba52a  Replace a!=b by not(a==b) in simplify_sqrt_real to cope with the change of logic induced by #19312 (Sage 6.10.beta7)

e2c7ab3  Scalar fields on the refactored topoological manifolds

cb53417  Add argument full_name to AbstractNamedObject; remove _repr_() from all parent classes; improve documentation

ce03503  Scalar fields on topological manifolds: slight improvements in the documentation

bc77059  Morphisms on the refactored topoological manifolds

2df7af7  Refactorization of differentiable manifolds, with the mixin class DifferentiableMixin

ecfbf2c  Tensor fields on refactored differentiable manifolds

comment:26 Changed 6 years ago by
 Commit changed from ecfbf2cd37624907f4d009d3096abd2b9c2312a9 to 8b720a5db590ed511f424fafc92e349f6de2f768
Branch pushed to git repo; I updated commit sha1. New commits:
c38ae80  Suppress the (unused) argument category in OpenTopologicalSubmanifold; minor doc improvements

069baf4  Merge branch 'public/manifolds/top_manif_basics' into Sage 7.0.beta2.

19caedb  Revert to AbstractNamedObject without argument full_name; restore _repr_() in manifold classes

9d25aa0  Scalar field algebras with AbstractNamedObject without argument full_name

ed4aa58  ScalarFieldAlgebra does not longer inherit from AbstractNamedObject

f6c6a34  Topological manifold morphisms with AbstractNamedObject without full_name

7204572  Basics of diff. manifolds with AbstractNamedObject without full_name

8b720a5  Tensor fields with AbstractNamedObject without full_name

comment:27 Changed 6 years ago by
 Commit changed from 8b720a5db590ed511f424fafc92e349f6de2f768 to 1f226bdceacae88627a26e24166d4bf6cf51b021
Branch pushed to git repo; I updated commit sha1. New commits:
3cd03a4  Add methods lift() and retract() to ManifoldSubset; add method __eq__() in CoordChange

984c3c2  Revert to simple hierarchy for manifold classes

c01048f  Scalar fields with the simplified hierarchy for manifold classes

c866d6c  Morphisms of topological manifolds with the simplified hierarchy for manifold classes

0e04631  Basics of diff. manifolds with the simplified hierarchy for manifold classes

1f226bd  Tensor fields with the simplified hierarchy for manifold classes

comment:28 Changed 6 years ago by
 Milestone changed from sage6.10 to sage7.0
comment:29 Changed 5 years ago by
 Commit changed from 1f226bdceacae88627a26e24166d4bf6cf51b021 to 2fad9db0007feee5a4b0f64eb4ec64b35dee1f74
Branch pushed to git repo; I updated commit sha1. New commits:
8e17d54  Merge into the latest version of #18529; improve treatment of composite functions in ExpressionNice

f00be00  Topological manifold morphisms: solved merge conflict with Sage 7.1.beta1

21b3968  Basics of diff. manifolds: solve merge conflict with Sage 7.1.beta1

8ba4b91  Tensor fields: solve merge conflict with Sage 7.1.beta1

2fad9db  Correct doctest in class DiffFormParal

comment:30 Changed 5 years ago by
 Commit changed from 2fad9db0007feee5a4b0f64eb4ec64b35dee1f74 to 73b4a036e94099b6e2b368f007be82bba127c797
Branch pushed to git repo; I updated commit sha1. Last 10 new commits:
57b7b7b  Implementing global_options for display formatting.

f11df50  Set CoordFunctionSymbRing in the category of commutative algebras over SR; minor documentation changes

f3f5470  Fixing a (essentially trivial) doctest failure.

7b3dab3  Merge branch 'public/manifolds/top_manif_scalar_fields' of git://trac.sagemath.org/sage into Sage 7.2.rc0

9ec7d3e  Python 3 format for print in manifolds

d190b37  Morphisms of topological manifolds with coordinate functions as algebra elements

2577fc5  Fixing a (essentially trivial) doctest failure.

7125e29  Python3 format for print in morphims of topological manifolds

bd2f35a  Diff. manifolds with coordinate functions as algebra elements

73b4a03  Tensor fields with with coordinate functions as algebra elements

comment:31 Changed 5 years ago by
 Milestone changed from sage7.0 to sage7.2
comment:32 Changed 5 years ago by
 Commit changed from 73b4a036e94099b6e2b368f007be82bba127c797 to 29832ece0fa3114f9a936ea1ebd0825305322698
Branch pushed to git repo; I updated commit sha1. New commits:
4fad094  Merge branch 'public/manifolds/top_manif_morphisms' of trac.sagemath.org:sage into public/manifolds/top_manif_morphisms

cfecb18  Reviewer changes and tweaks for continuous maps ticket.

efcb618  Modify authorship for continuous maps.

84051b0  Merge branch 'public/manifolds/diff_manif_basics' of trac.sagemath.org:sage into public/manifolds/diff_manif_basics

97172dd  Basics of differentiable manifolds with changes in morphisms of topological manifolds

29832ec  Use @cached_method for VectorFieldModule.identity_map() and AutomorphismFieldGroup.one()

comment:33 Changed 5 years ago by
 Commit changed from 29832ece0fa3114f9a936ea1ebd0825305322698 to 2150a43db88da8007dd6b35b1a5c6876e1c67eab
Branch pushed to git repo; I updated commit sha1. New commits:
2150a43  Correct doctests in vector fields after the merge of #20770 in sage 7.3.beta3

comment:34 Changed 5 years ago by
 Dependencies changed from #15916, #18783 to #15916, #18783, #20770
 Milestone changed from sage7.2 to sage7.3
comment:35 followups: ↓ 36 ↓ 45 Changed 5 years ago by
 Commit changed from 2150a43db88da8007dd6b35b1a5c6876e1c67eab to 3ab0af048e9ba51f09752a02f0d6544a53ffe15e
 Reviewers set to Travis Scrimshaw
 Status changed from needs_review to needs_work
Okay, here's the first round. Overall, the code looks okay, but I haven't check things in detail yet. I've made my way through most of the doc; there are likely some other doc fixes that I will need to do, but I've done a fair bit of cleanup. However, there are some issues:
 To run the tests on
tensorfield.py
takes ~172 seconds (that is after marking a bunch of them as# long time
). This is too long IMO, and we need to either find a way to speed things up or simplify the tests somehow (smaller manifolds perhaps?) so they don't take so long. One or two nontrivial toplevel examples are good. For the doctests, it just needs to be small examples showing the functionality. For any more complicated examples needed to do a more complete test should either be relatively small (in terms of number of lines) and be marked as# long time
or you need to have a separate file with the complete examples where most (all?) tests are marked as# long time
.
 I'm not sure about importing
xder
into the global namespace.exterior_derivative
I am okay with since it is explicit, butxder
is not a descriptive name IMO. If you want it, then you can import it in yourinit.sage
file (in the~/.sage
folder).
 I added some long versions of each of the names. It is good to at least have the long name. I actually changed
exterior_der
toexterior_derivative
. I am not opposed to having an alias toxder
though.
 While in general, it is good to adhere to 80 character lines, sometimes it just makes it harder read (which is why it is a guideline). I exercised some judgment and made some lines go over this because IMO they become easier to read.
 We will probably want to use the coercion framework, so
__mul__
which handles the scalar action should becomeacted_upon
or_rmul_
(and_lmul_
).
I believe this is the biggest of the remaining SageManifolds tickets, so we just need to power through this and the rest should be easy...
comment:36 in reply to: ↑ 35 ; followup: ↓ 37 Changed 5 years ago by
Replying to tscrim:
Okay, here's the first round.
Thank you very much for reviewing this!
Overall, the code looks okay, but I haven't check things in detail yet. I've made my way through most of the doc; there are likely some other doc fixes that I will need to do, but I've done a fair bit of cleanup. However, there are some issues:
 To run the tests on
tensorfield.py
takes ~172 seconds (that is after marking a bunch of them as# long time
). This is too long IMO, and we need to either find a way to speed things up or simplify the tests somehow (smaller manifolds perhaps?) so they don't take so long. One or two nontrivial toplevel examples are good. For the doctests, it just needs to be small examples showing the functionality. For any more complicated examples needed to do a more complete test should either be relatively small (in terms of number of lines) and be marked as# long time
or you need to have a separate file with the complete examples where most (all?) tests are marked as# long time
.
OK I will try to shorten significantly the doctests.
 I'm not sure about importing
xder
into the global namespace.exterior_derivative
I am okay with since it is explicit, butxder
is not a descriptive name IMO. If you want it, then you can import it in yourinit.sage
file (in the~/.sage
folder).
I agree xder
is not as descriptive as exterior_derivative
. Certainly the latter should be the primary function name. However for the end user manipulating a lot of differential forms, it is certainly desirable to have a short name (in addition to TAB completion), as diff
is a short name for derivative
. We may even think of a shorter name, like xd
, as a reminder of the standard mathematical notation d.
 I added some long versions of each of the names. It is good to at least have the long name. I actually changed
exterior_der
toexterior_derivative
. I am not opposed to having an alias toxder
though.
 While in general, it is good to adhere to 80 character lines, sometimes it just makes it harder read (which is why it is a guideline). I exercised some judgment and made some lines go over this because IMO they become easier to read.
OK.
 We will probably want to use the coercion framework, so
__mul__
which handles the scalar action should becomeacted_upon
or_rmul_
(and_lmul_
).
Yes, this seems desirable. I'll try it.
I believe this is the biggest of the remaining SageManifolds tickets,
Yes, certainly.
so we just need to power through this and the rest should be easy...
comment:37 in reply to: ↑ 36 ; followup: ↓ 38 Changed 5 years ago by
Replying to egourgoulhon:
Replying to tscrim:
Okay, here's the first round.
Thank you very much for reviewing this!
No problem. Sorry its taken so long. Hopefully I can make it through this soon.
 I'm not sure about importing
xder
into the global namespace.exterior_derivative
I am okay with since it is explicit, butxder
is not a descriptive name IMO. If you want it, then you can import it in yourinit.sage
file (in the~/.sage
folder).I agree
xder
is not as descriptive asexterior_derivative
. Certainly the latter should be the primary function name. However for the end user manipulating a lot of differential forms, it is certainly desirable to have a short name (in addition to TAB completion), asdiff
is a short name forderivative
. We may even think of a shorter name, likexd
, as a reminder of the standard mathematical notation d.
We currently have no differential
function, just its shorten diff
, which is the Sage equivalent for derivative
. I am not opposed to having an xder
alias, just importing it into the global namespace (by default) as it is not a "standard" short name. However, you could include the import of xder
as part of the examples/tutorials. I do think that xder
is okay as a method alias for exterior_derivative
as well.
IMO xd
, like d
, is too short and generic to be useful at the toplevel.
comment:38 in reply to: ↑ 37 ; followup: ↓ 39 Changed 5 years ago by
Replying to tscrim:
We currently have no
differential
function, just its shortendiff
, which is the Sage equivalent forderivative
. I am not opposed to having anxder
alias, just importing it into the global namespace (by default) as it is not a "standard" short name. However, you could include the import ofxder
as part of the examples/tutorials.
I agree that the exterior derivative is too specialized to have it in the global namespace of any sage session. On the other side, end users working with differential forms may feel cumbersome to perform some import in each of their sessions. A solution could be to modify the top function Manifold
so that it imports xder
into the global namespace. Is such a thing possible/desirable?
comment:39 in reply to: ↑ 38 ; followup: ↓ 40 Changed 5 years ago by
Replying to egourgoulhon:
Replying to tscrim:
We currently have no
differential
function, just its shortendiff
, which is the Sage equivalent forderivative
. I am not opposed to having anxder
alias, just importing it into the global namespace (by default) as it is not a "standard" short name. However, you could include the import ofxder
as part of the examples/tutorials.I agree that the exterior derivative is too specialized to have it in the global namespace of any sage session. On the other side, end users working with differential forms may feel cumbersome to perform some import in each of their sessions. A solution could be to modify the top function
Manifold
so that it importsxder
into the global namespace. Is such a thing possible/desirable?
This is what the init.sage
is for. It is called every time Sage is loaded. I feel that one import to setup an environment with a convenience function is very little to ask. IMO, it is not good for the entry point Manifold
to have side effects.
comment:40 in reply to: ↑ 39 ; followups: ↓ 41 ↓ 42 Changed 5 years ago by
Replying to tscrim:
Replying to egourgoulhon:
I agree that the exterior derivative is too specialized to have it in the global namespace of any sage session. On the other side, end users working with differential forms may feel cumbersome to perform some import in each of their sessions. A solution could be to modify the top function
Manifold
so that it importsxder
into the global namespace. Is such a thing possible/desirable?This is what the
init.sage
is for. It is called every time Sage is loaded.
What about the SageMathCloud? It's certainly technically possible to define some init.sage
there, but one has to open a terminal and edit a file, etc.
Also a drawback of init.sage
is that your worksheets cannot easily be shared with somebody else, or made publicly available for repeating the calculation.
I feel that one import to setup an environment with a convenience function is very little to ask. IMO, it is not good for the entry point
Manifold
to have side effects.
What about Manifold.import_convenience_functions(verbose=True)
or something like this? It could
(i) put functions like xder
in the global namespace and (ii) print out the list of such functions if verbose
is True
.
comment:41 in reply to: ↑ 40 Changed 5 years ago by
Replying to egourgoulhon:
What about
Manifold.import_convenience_functions(verbose=True)
or something like this? It could (i) put functions likexder
in the global namespace and (ii) print out the list of such functions ifverbose
isTrue
.
Granted: from sage.manifolds.utilities import xder
is as short to write...
comment:42 in reply to: ↑ 40 ; followup: ↓ 43 Changed 5 years ago by
Replying to egourgoulhon:
Replying to tscrim:
Replying to egourgoulhon:
I agree that the exterior derivative is too specialized to have it in the global namespace of any sage session. On the other side, end users working with differential forms may feel cumbersome to perform some import in each of their sessions. A solution could be to modify the top function
Manifold
so that it importsxder
into the global namespace. Is such a thing possible/desirable?This is what the
init.sage
is for. It is called every time Sage is loaded.What about the SageMathCloud? It's certainly technically possible to define some
init.sage
there, but one has to open a terminal and edit a file, etc.Also a drawback of
init.sage
is that your worksheets cannot easily be shared with somebody else, or made publicly available for repeating the calculation.
For SMC worksheets, you can easily have the setup in an %auto
block.
I feel that one import to setup an environment with a convenience function is very little to ask. IMO, it is not good for the entry point
Manifold
to have side effects.What about
Manifold.import_convenience_functions(verbose=True)
or something like this? It could (i) put functions likexder
in the global namespace and (ii) print out the list of such functions ifverbose
isTrue
.
That is no different than doing a usual import. I guess a question that I have is how many such functions do you think there will eventually be?
comment:43 in reply to: ↑ 42 Changed 5 years ago by
Replying to tscrim:
What about
Manifold.import_convenience_functions(verbose=True)
or something like this? It could (i) put functions likexder
in the global namespace and (ii) print out the list of such functions ifverbose
isTrue
.That is no different than doing a usual import.
Yes I realized this just after posting my comment...
I guess a question that I have is how many such functions do you think there will eventually be?
Not much... So let's stay with the usual import ;)
comment:44 Changed 5 years ago by
 Commit changed from 3ab0af048e9ba51f09752a02f0d6544a53ffe15e to dcb08fc969128ca01e9c5bb88ec1cd93aba334ff
comment:45 in reply to: ↑ 35 Changed 5 years ago by
Replying to tscrim:
 To run the tests on
tensorfield.py
takes ~172 seconds (that is after marking a bunch of them as# long time
). This is too long IMO, and we need to either find a way to speed things up or simplify the tests somehow (smaller manifolds perhaps?) so they don't take so long. One or two nontrivial toplevel examples are good. For the doctests, it just needs to be small examples showing the functionality. For any more complicated examples needed to do a more complete test should either be relatively small (in terms of number of lines) and be marked as# long time
or you need to have a separate file with the complete examples where most (all?) tests are marked as# long time
.
By selecting simpler examples, the last commit decreases by a factor ~2 the doctest time of tensorfield.py
. It's difficult to go below this because the manifold dimension is already at its minimum (for a nonparallelizable manifold): it is 2 for all tests.
 I'm not sure about importing
xder
into the global namespace.exterior_derivative
I am okay with since it is explicit, butxder
is not a descriptive name IMO. If you want it, then you can import it in yourinit.sage
file (in the~/.sage
folder).
xder
is no longer in the global namespace. I've also suppressed exterior_derivative
, but maybe we can restore the latter, as you suggest.
 I added some long versions of each of the names. It is good to at least have the long name. I actually changed
exterior_der
toexterior_derivative
. I am not opposed to having an alias toxder
though.
In the same spirit, I have renamed lie_der
to lie_derivative
and set lie_der
as an alias to it.
 We will probably want to use the coercion framework, so
__mul__
which handles the scalar action should becomeacted_upon
or_rmul_
(and_lmul_
).
Actually _rmul_
is already implemented in tensor fields and handles the scalar action. The current __mul__
handles mostly the tensor product. Since this is an external operation (w.r.t. the parent module), I am not sure that it can be changed to some single underscore method within the coercion framework. Note that we already had __mul__
for handling tensor products in sage.tensor.modules.free_module_tensor.FreeModuleTensor
.
comment:46 Changed 5 years ago by
 Status changed from needs_work to needs_review
comment:47 Changed 5 years ago by
I checked out this commit with 7.4 beta0. All of my Jupyter worksheets that use forms and vectors seem to work fine. I would like to encourage you to include this functionality in Sage in the next possible release. I would give this a positive review except that the commit is still based on an older release of Sage and that milestone has passed.
comment:48 Changed 5 years ago by
 Cc bpage added
comment:49 followup: ↓ 50 Changed 5 years ago by
I am (still) making my way through my second pass of the code and doc, but hopefully soon I will have finished.
comment:50 in reply to: ↑ 49 Changed 5 years ago by
Replying to tscrim:
I am (still) making my way through my second pass of the code and doc, but hopefully soon I will have finished.
Thanks for this update and your work on this.
Replying to bpage: good to know that all your Jupyter worksheets passed. However, we should really wait for Travis detailed review before concluding with this ticket.
comment:51 Changed 5 years ago by
 Commit changed from dcb08fc969128ca01e9c5bb88ec1cd93aba334ff to 7cf4ff47794e551141bf475efa2accdaefda3d53
Branch pushed to git repo; I updated commit sha1. New commits:
c58924f  Merge branch 'develop' into public/manifolds/diff_manif_tensor_fields

27a369e  Merge branch 'develop' into public/manifolds/diff_manif_tensor_fields

13cb850  Merge branch 'develop' into public/manifolds/diff_manif_tensor_fields

88295c6  Merge branch 'develop' into public/manifolds/diff_manif_tensor_fields

58f7fb9  Merge branch 'develop' into public/manifolds/diff_manif_tensor_fields

3e73095  Merge branch 'develop' into public/manifolds/diff_manif_tensor_fields

4955c3c  Final review of everything.

7cf4ff4  Merge branch 'public/manifolds/diff_manif_tensor_fields' of trac.sagemath.org:sage into public/manifolds/diff_manif_tensor_fields

comment:52 followup: ↓ 54 Changed 5 years ago by
I (finally) made it through the code again, and most of my changes were smaller documentation cleanups and fixes. I changed some custom caching into @cached_method
(IIRC it is faster) and separated out the zero element into zero
(with @cached_method
). If you're good with my changes, then we can set this to positive review (and hopefully sneak this into 7.4).
comment:53 Changed 5 years ago by
 Commit changed from 7cf4ff47794e551141bf475efa2accdaefda3d53 to a841f794bb2bc29fd8af4b212e6e2a93b5c58057
Branch pushed to git repo; I updated commit sha1. New commits:
58ae506  Merge branch 'public/manifolds/diff_manif_tensor_fields' of git://trac.sagemath.org/sage into Sage 7.4.beta6

8219403  Fix documentation error; change in TensorField comparison to zero

a841f79  Python 3 compatible syntax in tensor fields

comment:54 in reply to: ↑ 52 Changed 5 years ago by
Replying to tscrim:
I (finally) made it through the code again, and most of my changes were smaller documentation cleanups and fixes. I changed some custom caching into
@cached_method
(IIRC it is faster) and separated out the zero element intozero
(with@cached_method
).
Thank you very much for your review and the changes in the code!
I've committed a few minor changes: besides fixing some documentation error (missing blank line),
 this reverts to the previous version of comparison to zero in
TensorField.__eq__
: indeed the following code in commit 7cf4ff4if other == 0: return self.is_zero() elif other in ZZ: return False
could return True fora == b
witha
being a zero vector, say, andb
being a zero 2form. We do not want this behavior, do we? We should only allow for the possibility to writea == 0
as a shortcut fora.is_zero()
.  this changes some syntax to make the code compatible with Python 3 (in view of the future migration): all the
.itervalues()
and.iteritems()
have been replaced by.values()
and.items()
respectively. I've not used the functionsitervalues
anditeritems
fromsix
, since all dictionaries are small and changing from an iterator to a list (in the Python 2.7 version) should not make a big difference (for instance, I've checked that there is no change in the doctest time).
If you're good with my changes, then we can set this to positive review (and hopefully sneak this into 7.4).
It's OK for me ;)
comment:55 followup: ↓ 57 Changed 5 years ago by
I'm okay with your changes modulo the straight revert to this isinstance(other, (int, Integer))
. What if other = QQ.zero()
? A better check (if slightly more in terms of time) is other in ZZ
:
sage: vector([0,0,0]) == 0 True sage: vector([0,0,0]) in ZZ False
Although in principle, the comparison a == b
should convert to a common parent...
comment:56 Changed 5 years ago by
 Commit changed from a841f794bb2bc29fd8af4b212e6e2a93b5c58057 to 5a5f4008bf54a7b6e71c71d13ba7aa10db7e75e6
Branch pushed to git repo; I updated commit sha1. New commits:
5a5f400  Slight change in TensorField.__eq__ (comparison to zero)

comment:57 in reply to: ↑ 55 Changed 5 years ago by
Replying to tscrim:
A better check (if slightly more in terms of time) is
other in ZZ
:
OK, I've implemented it in the last commit.
comment:58 followup: ↓ 59 Changed 5 years ago by
 Milestone changed from sage7.3 to sage7.4
 Status changed from needs_review to positive_review
Thanks.
comment:59 in reply to: ↑ 58 Changed 5 years ago by
Thank you for the detailed review work and the resulting enhancement of the code!
comment:60 followup: ↓ 62 Changed 5 years ago by
 Status changed from positive_review to needs_work
[dochtml] OSError: [manifolds] /mnt/disk/home/release/Sage/local/lib/python2.7/sitepackages/sage/manifolds/chart.py:docstring of sage.manifolds.chart:21: WARNING: citation not found: Lee13
comment:61 Changed 5 years ago by
 Commit changed from 5a5f4008bf54a7b6e71c71d13ba7aa10db7e75e6 to 8860387a6637d8c0d117c8fcfcf1524fcee6ff1c
Branch pushed to git repo; I updated commit sha1. New commits:
8860387  Add missing reference Lee13

comment:62 in reply to: ↑ 60 Changed 5 years ago by
 Status changed from needs_work to needs_review
Replying to vbraun:
[dochtml] OSError: [manifolds] /mnt/disk/home/release/Sage/local/lib/python2.7/sitepackages/sage/manifolds/chart.py:docstring of sage.manifolds.chart:21: WARNING: citation not found: Lee13
Oups! Sorry about this. The last commit adds the missing reference.
comment:63 Changed 5 years ago by
 Status changed from needs_review to positive_review
Having checked that, after a make docclean && make doc, the issue is fixed, I am taking the liberty of setting the ticket back to posivite review.
comment:64 followup: ↓ 66 Changed 5 years ago by
 Status changed from positive_review to needs_work
Conflicts with #21454
comment:65 Changed 5 years ago by
 Commit changed from 8860387a6637d8c0d117c8fcfcf1524fcee6ff1c to fb7f4dd6e483f9956f49475bc16707f739f1129a
Branch pushed to git repo; I updated commit sha1. New commits:
fb7f4dd  Bibliographic references for tensor fields moved to the master file

comment:66 in reply to: ↑ 64 Changed 5 years ago by
 Status changed from needs_work to needs_review
comment:67 followup: ↓ 68 Changed 5 years ago by
 Status changed from needs_review to positive_review
comment:68 in reply to: ↑ 67 Changed 5 years ago by
Replying to tscrim:
Thanks!
comment:69 Changed 5 years ago by
 Branch changed from public/manifolds/diff_manif_tensor_fields to fb7f4dd6e483f9956f49475bc16707f739f1129a
 Resolution set to fixed
 Status changed from positive_review to closed
Branch pushed to git repo; I updated commit sha1. New commits:
Start to improve documentation on tensor fields on differentiable manifolds