Opened 7 years ago
Closed 6 years ago
#18843 closed enhancement (fixed)
Differentiable manifolds: vector fields and tensor fields
Reported by: | egourgoulhon | Owned by: | egourgoulhon |
---|---|---|---|
Priority: | major | Milestone: | sage-7.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 follow-up of #18783 within the SageManifolds project (see the metaticket #18528 for an overview). As in #18783, the non-discrete 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 (p-forms) on a differentiable manifoldDiffFormFreeModule
: free module of differential forms of a given degree p (p-forms) 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
: p-form on differentiable manifoldAutomorphismField
: field of tangent-space automorphisms on a differentiable manifold
TensorFieldParal
: tensor field on a parallelizable differentiable manifoldVectorFieldParal
: vector field on a parallelizable differentiable manifoldDiffFormParal
: p-form on parallelizable differentiable manifoldAutomorphismFieldParal
: field of tangent-space 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 1-forms) 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 7 years ago by
- Commit changed from d48210c11f38f90ce656f0fa25ec550d147a1892 to b9c3bc529cdd9e7c11f5f6bd10f6e29b070b7cbc
comment:2 Changed 7 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 7 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 7 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 7 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 7 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 7 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 7 years ago by
- Description modified (diff)
comment:9 Changed 7 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 7 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 7 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 7 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 7 years ago by
- Dependencies changed from #15916, #18100, #18783 to #15916, #18783
comment:14 Changed 7 years ago by
- Milestone changed from sage-6.8 to sage-6.9
comment:15 Changed 7 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 7 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 7 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 non-verbose
|
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 7 years ago by
- Description modified (diff)
- Milestone changed from sage-6.9 to sage-6.10
- Status changed from new to needs_review
comment:19 Changed 7 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 p-adics 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 7 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 7 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 7 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 7 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 7 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 pynac-0.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 7 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 7 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 sage-6.10 to sage-7.0
comment:29 Changed 6 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 6 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 6 years ago by
- Milestone changed from sage-7.0 to sage-7.2
comment:32 Changed 6 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 6 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 6 years ago by
- Dependencies changed from #15916, #18783 to #15916, #18783, #20770
- Milestone changed from sage-7.2 to sage-7.3
comment:35 follow-ups: ↓ 36 ↓ 45 Changed 6 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 clean-up. 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 non-trivial top-level 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 ; follow-up: ↓ 37 Changed 6 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 clean-up. 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 non-trivial top-level 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 ; follow-up: ↓ 38 Changed 6 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 top-level.
comment:38 in reply to: ↑ 37 ; follow-up: ↓ 39 Changed 6 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 ; follow-up: ↓ 40 Changed 6 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 ; follow-ups: ↓ 41 ↓ 42 Changed 6 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 6 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 ; follow-up: ↓ 43 Changed 6 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 6 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 6 years ago by
- Commit changed from 3ab0af048e9ba51f09752a02f0d6544a53ffe15e to dcb08fc969128ca01e9c5bb88ec1cd93aba334ff
comment:45 in reply to: ↑ 35 Changed 6 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 non-trivial top-level 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 non-parallelizable 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 6 years ago by
- Status changed from needs_work to needs_review
comment:47 Changed 6 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 6 years ago by
- Cc bpage added
comment:49 follow-up: ↓ 50 Changed 6 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 6 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 Travis detailed review before concluding with this ticket.
comment:51 Changed 6 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 follow-up: ↓ 54 Changed 6 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 6 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 6 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 2-form. 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 follow-up: ↓ 57 Changed 6 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 6 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 6 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 follow-up: ↓ 59 Changed 6 years ago by
- Milestone changed from sage-7.3 to sage-7.4
- Status changed from needs_review to positive_review
Thanks.
comment:59 in reply to: ↑ 58 Changed 6 years ago by
Thank you for the detailed review work and the resulting enhancement of the code!
comment:60 follow-up: ↓ 62 Changed 6 years ago by
- Status changed from positive_review to needs_work
[dochtml] OSError: [manifolds] /mnt/disk/home/release/Sage/local/lib/python2.7/site-packages/sage/manifolds/chart.py:docstring of sage.manifolds.chart:21: WARNING: citation not found: Lee13
comment:61 Changed 6 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 6 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/site-packages/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 6 years ago by
- Status changed from needs_review to positive_review
Having checked that, after a make doc-clean && make doc, the issue is fixed, I am taking the liberty of setting the ticket back to posivite review.
comment:64 follow-up: ↓ 66 Changed 6 years ago by
- Status changed from positive_review to needs_work
Conflicts with #21454
comment:65 Changed 6 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 6 years ago by
- Status changed from needs_work to needs_review
comment:67 follow-up: ↓ 68 Changed 6 years ago by
- Status changed from needs_review to positive_review
comment:68 in reply to: ↑ 67 Changed 6 years ago by
Replying to tscrim:
Thanks!
comment:69 Changed 6 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