Differentiable manifolds: vector fields and tensor fields

[egourgoulhon]

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 manifold
• VectorFieldFreeModule: free module of vector fields on a parallelizable differentiable manifold
• TensorFieldModule: module of tensor fields of a given type (k,l) on a differentiable manifold
• TensorFieldFreeModule: free module of tensor fields of a given type (k,l) on a parallelizable differentiable manifold
• DiffFormModule: module of differential forms of a given degree p (p-forms) on a differentiable manifold
• DiffFormFreeModule: free module of differential forms of a given degree p (p-forms) on a parallelizable differentiable manifold
• AutomorphismFieldGroup: general linear group of the module of vector fields on a differentiable manifold
• AutomorphismFieldParalGroup: general linear group of the free module of vector fields on a parallelizable differentiable manifold

2/ Element classes:

• TensorField: tensor field on a differentiable manifold
  • VectorField: vector field on a differentiable manifold
  • DiffForm: p-form on differentiable manifold
  • AutomorphismField: field of tangent-space automorphisms on a differentiable manifold
• TensorFieldParal: tensor field on a parallelizable differentiable manifold
  • VectorFieldParal: vector field on a parallelizable differentiable manifold
  • DiffFormParal: p-form on parallelizable differentiable manifold
  • AutomorphismFieldParal: field of tangent-space automorphisms on a parallelizable differentiable manifold

3/ Other classes:

• VectorFrame: vector frame on a differentiable manifold
  • CoordFrame: coordinate vector frame on a differentiable manifold
• CoFrame: coframe (frame of 1-forms) on a differentiable manifold
  • CoordCoFrame: 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.

[git]

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

​b9c3bc5

Start to improve documentation on tensor fields on differentiable manifolds

[git]

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

​a2e0696

Add method display() to tensor components and display_comp() to tensors

[git]

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

​cad56b1

Improve documentation of tensor fields

[git]

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

​794c149

Improve documentation of differential forms

[git]

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

​435fb11

Improve documentation of vector frames

[git]

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

​8f47f64

More improvements in the documentation of diff. manifolds

[git]

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

​935a0f7

Improvements in the documentation of class DiffManifold

[git]

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

​2c6b711

Improve documentation of scalar and vector fields; add doctests

[git]

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

​eeff027

Add doctests for tensor fields

[git]

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

​686cee1

Full doctest coverage.

[git]

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()

[git]

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

​307e655

Major improvements in the documentation of diff. manifolds (tensor field part)

[git]

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

​dc7f7a1

Improve TensorField.__eq__ (case with no open cover known)

[git]

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

[git]

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)

[git]

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

[git]

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

​e8f11ff

Fix pickling test in tensor field modules.

[git]

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

​a967787

Suppress direct call to _element_constructor_ in tensor field parent classes

[git]

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

[git]

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)

[git]

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

[git]

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

[git]

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

[git]

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

[git]

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

[git]

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()

[git]

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

[tscrim]

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, but xder is not a descriptive name IMO. If you want it, then you can import it in your init.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 to exterior_derivative. I am not opposed to having an alias to xder 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 become acted_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...

[egourgoulhon]

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, but xder is not a descriptive name IMO. If you want it, then you can import it in your init.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 to exterior_derivative. I am not opposed to having an alias to xder 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 become acted_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...

[tscrim]

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, but xder is not a descriptive name IMO. If you want it, then you can import it in your init.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.

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.

[egourgoulhon]

Replying to tscrim:

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 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?

[tscrim]

Replying to egourgoulhon:

Replying to tscrim:

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 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?

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.

[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 imports xder 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.

[egourgoulhon]

Replying to egourgoulhon:

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.

Granted: from sage.manifolds.utilities import xder is as short to write...

[tscrim]

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 imports xder 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 like xder in the global namespace and (ii) print out the list of such functions if verbose is True.

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?

[egourgoulhon]

Replying to tscrim:

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.

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 ;-)

[git]

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

​d7ad8e1	Merge branch 'public/manifolds/diff_manif_tensor_fields' of git://trac.sagemath.org/sage into sage 7.3.beta5
​dcb08fc	Decrease doctest times in tensor fields; remove xder from the global namespace

[egourgoulhon]

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, but xder is not a descriptive name IMO. If you want it, then you can import it in your init.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 to exterior_derivative. I am not opposed to having an alias to xder 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 become acted_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.

[bpage]

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.

[tscrim]

I am (still) making my way through my second pass of the code and doc, but hopefully soon I will have finished.

[egourgoulhon]

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.

[git]

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

[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 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).

[git]

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

[egourgoulhon]

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 into zero (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 7cf4ff4

   if other == 0:
       return self.is_zero()
   elif other in ZZ:
       return False
  

  could return True for a == b with a being a zero vector, say, and b being a zero 2-form. We do not want this behavior, do we? We should only allow for the possibility to write a == 0 as a shortcut for a.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 functions itervalues and iteritems from six, 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 ;-)

[tscrim]

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...

[git]

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

​5a5f400

Slight change in TensorField.__eq__ (comparison to zero)

[egourgoulhon]

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.

[tscrim]

Thanks.

[egourgoulhon]

Thank you for the detailed review work and the resulting enhancement of the code!

[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

[git]

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

​8860387

Add missing reference Lee13

[egourgoulhon]

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.

[egourgoulhon]

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.

[vbraun]

Conflicts with #21454

[git]

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

​fb7f4dd

Bibliographic references for tensor fields moved to the master file

[egourgoulhon]

Replying to vbraun:

Conflicts with #21454

Fixed!

[egourgoulhon]

Replying to tscrim:

Thanks!