# Simplify VectorField.__call__

Reported by: Owned by: egourgoulhon major sage-9.4 manifolds vector field, scalar field tscrim, gh-mjungmath Eric Gourgoulhon Michael Jung N/A bc25781 bc257815fcd42a6ba49eb0639ad1dc7e6a172f03

### Description

The method VectorField.__call__ implements the action v(f) of a vector field v on a scalar field f, as a derivation. It is reimplemented here to simply return df(v), i.e. the differential of f acting on v as a 1-form. Actually, the current code of this method contains the comment:

        #!# Could it be simply
# return scalar.differential()(self)
# ?


git blame reveals that this comment dates back to 2015. It is time to make the change, thereby simplifying the code and avoiding some code duplication.

### comment:1 Changed 14 months ago by egourgoulhon

• Branch set to public/manifolds/vector_field_call_31633
• Commit set to c921e9738ac5d828eec7f8a0b0cbd60eba81a9f0
• Status changed from new to needs_review

New commits:

 ​c921e97 Simplify VectorField.__call__

### comment:2 follow-up: ↓ 4 Changed 14 months ago by gh-mjungmath

Nice! It's good to reduce code and get rid of duplication!

Two things:

• What exactly caused the change
     sage: isinstance(Tp, FiniteRankFreeModule)
True
sage: sorted(Tp.bases(), key=str)
-    [Basis (d/dr,d/dph) on the Tangent space at Point p on the Euclidean plane E^2,
-     Basis (e_r,e_ph) on the Tangent space at Point p on the Euclidean plane E^2,
+    [Basis (e_r,e_ph) on the Tangent space at Point p on the Euclidean plane E^2,
Basis (e_x,e_y) on the Tangent space at Point p on the Euclidean plane E^2]

• It is possible to combine raw strings and f-strings:
-                latex_name = r"{}\left({}\right)".format(self._latex_name,
-                                                         scalar._latex_name)
+                latex_name = fr"{self._latex_name}\left({scalar._latex_name}\right)"


### comment:3 Changed 14 months ago by git

• Commit changed from c921e9738ac5d828eec7f8a0b0cbd60eba81a9f0 to bc257815fcd42a6ba49eb0639ad1dc7e6a172f03

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

 ​bc25781 Combine raw string and f-string in VectorField.__call__

### comment:4 in reply to: ↑ 2 ; follow-up: ↓ 5 Changed 14 months ago by egourgoulhon

• What exactly caused the change
     sage: isinstance(Tp, FiniteRankFreeModule)
True
sage: sorted(Tp.bases(), key=str)
-    [Basis (d/dr,d/dph) on the Tangent space at Point p on the Euclidean plane E^2,
-     Basis (e_r,e_ph) on the Tangent space at Point p on the Euclidean plane E^2,
+    [Basis (e_r,e_ph) on the Tangent space at Point p on the Euclidean plane E^2,
Basis (e_x,e_y) on the Tangent space at Point p on the Euclidean plane E^2]


It's because in the previous VectorFieldParal.__call__ code, the computation was performed as vi df/dxi in all possible coordinate frames d/dxi, thereby triggering the evaluation of v in these frames. In the new code, the computation is performed as (df)i vi in a single frame (and not necessarily a coordinate one). In the specific example above, v(F) in line 577 of src/doc/en/thematic_tutorials/vector_calculus/vector_calc_plane.rst triggered the computation of v in the coordinate frame (d/dr, d/dph); this was done in line 1779 of the old (i.e. 9.3.rc2) file vectorfield.py:

                self_r.comp(chart._frame)


Then vp = v.at(p) in line 634 of vector_calc_plane.rst created the basis (d/dr, d/dph) in Tp; this was done by frame.at(point) in line 2109 of tensorfield_paral.py:

            comp_resu = resu.add_comp(frame.at(point))


This explains why Tp had the extra basis (d/dr, d/dph) with the old code. It was a side effect of an unnecessary calculation in the old VectorFieldParal.__call__.

• It is possible to combine raw strings and f-strings:
-                latex_name = r"{}\left({}\right)".format(self._latex_name,
-                                                         scalar._latex_name)
+                latex_name = fr"{self._latex_name}\left({scalar._latex_name}\right)"


Done in the last commit.

### comment:5 in reply to: ↑ 4 Changed 14 months ago by gh-mjungmath

It's because in the previous VectorFieldParal.__call__ code, the computation was performed as vi df/dxi in all possible coordinate frames d/dxi, thereby triggering the evaluation of v in these frames. In the new code, the computation is performed as (df)i vi in a single frame (and not necessarily a coordinate one). In the specific example above, v(F) in line 577 of src/doc/en/thematic_tutorials/vector_calculus/vector_calc_plane.rst triggered the computation of v in the coordinate frame (d/dr, d/dph); this was done in line 1779 of the old (i.e. 9.3.rc2) file vectorfield.py:

                self_r.comp(chart._frame)


Then vp = v.at(p) in line 634 of vector_calc_plane.rst created the basis (d/dr, d/dph) in Tp; this was done by frame.at(point) in line 2109 of tensorfield_paral.py:

            comp_resu = resu.add_comp(frame.at(point))


This explains why Tp had the extra basis (d/dr, d/dph) with the old code. It was a side effect of an unnecessary calculation in the old VectorFieldParal.__call__.

That's indeed an improvement. Sweet!

• It is possible to combine raw strings and f-strings:
-                latex_name = r"{}\left({}\right)".format(self._latex_name,
-                                                         scalar._latex_name)
+                latex_name = fr"{self._latex_name}\left({scalar._latex_name}\right)"


Done in the last commit.

Great. I'll give it a positive review as soon as patchbot is green.

### comment:6 Changed 14 months ago by gh-mjungmath

• Reviewers set to Michael Jung
• Status changed from needs_review to positive_review

Patchbot green. LGTM.

### comment:7 Changed 14 months ago by egourgoulhon

Thank you for the review!

### comment:8 Changed 12 months ago by vbraun

• Branch changed from public/manifolds/vector_field_call_31633 to bc257815fcd42a6ba49eb0639ad1dc7e6a172f03
• Resolution set to fixed
• Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.