Simplify VectorField.__call__

[egourgoulhon]

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.

[egourgoulhon]

New commits:

​c921e97

Simplify VectorField.__call__

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

[git]

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

​bc25781

Combine raw string and f-string in VectorField.__call__

[egourgoulhon]

Replying to gh-mjungmath:

Thanks for your comments.

• 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 vⁱ df/dxⁱ in all possible coordinate frames d/dxⁱ, thereby triggering the evaluation of v in these frames. In the new code, the computation is performed as (df)_i vⁱ 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.

[gh-mjungmath]

Replying to egourgoulhon:

It's because in the previous VectorFieldParal.__call__ code, the computation was performed as vⁱ df/dxⁱ in all possible coordinate frames d/dxⁱ, thereby triggering the evaluation of v in these frames. In the new code, the computation is performed as (df)_i vⁱ 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.

[gh-mjungmath]

Patchbot green. LGTM.

[egourgoulhon]

Thank you for the review!