Tensor Fields: Better Zero Treatment

[DeRhamSource]

The zero element is always a special element. Therefore it should be treated as such. It should shorten computations and certainly be immutable. This ticket is devoted to that topic. (Similarly for the one element in the scalar field and mixed form algebra).

This ticket is part of the metaticket #28519.

Features

• an assertion error arises when altering the fixed elements zero or one
• a new attribute _is_zero is added to tensor fields and mixed form (similar to scalar fields)
• computations with involved zero or one are shortened by using a simple check
• due to immutability of algebra elements, no copies are returned anymore for scalar field operations with zero or one
• _is_zero attribute is applied for copies

CC: @tscrim @egourgoulhon

Component: geometry

Keywords: tensor fields, scalar fields, mixed forms

Author: Michael Jung

Branch/Commit: f2928f8

Reviewer: Eric Gourgoulhon, Travis Scrimshaw

Issue created by migration from https://trac.sagemath.org/ticket/28562

[DeRhamSource]

Branch: u/gh-DeRhamSource/better_zero_treatment

[DeRhamSource]

New commits:

9f91503

'_is_zero' attribute added and modified

[DeRhamSource]

Commit: 9f91503

[sagetrac-git]

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

5094a3e

Return no copies for scalar field operations

[sagetrac-git]

Changed commit from 9f91503 to 5094a3e

[sagetrac-git]

Changed commit from 5094a3e to 94cf06a

[sagetrac-git]

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

ca8dd88	Modules modified
892050e	FreeModuleLinearGroup: one treatment
1b96587	Zero treatment for FreeModules
b9c8287	AutomorphismField: zero treatment + MixedForm: set_restriction and add_comp_by_continuation
94cf06a	Mixed forms adapted to zero treatment + zero treatment for scalar fields

[DeRhamSource]

comment:8

In MixedForm, the zero (and one element for scalar fields) is treated separately. The code in there is not very beautiful, but it works for now. However, I plan to reorganize the code of mixed forms anyway (see #28519).

[DeRhamSource]

comment:9

Also, I removed the _zero_element attribute in the free modules and replaced it by a cached zero() method. This is a better solution, I guess.

[sagetrac-git]

Changed commit from 94cf06a to d949a13

[sagetrac-git]

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

d949a13

Minor code reduction

[sagetrac-git]

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

5d30100

More code reduction

[sagetrac-git]

Changed commit from d949a13 to 5d30100

[DeRhamSource]

comment:12

The code snippet (see commits 5d30100 and ​d949a13) was quite messy. Well, it still is. At least it is readable and works for now. It gets a little bit better after merging (see #28519).

I will think about a better solution. But I guess, this requires a complete reorganization of the mixed form code.

[DeRhamSource]

comment:13

After a bit of thinking, I came to the conclusion that the code snippet should be obsolete. (As a mathematician, I feel the urge to solve or fix a problem/error as soon as it occurs. Here: Doctest errors.) The thing is, due to our little discussion in #28519, the doctest must be flawed since mixed forms defined via preconstructed forms get altered.

So, I 'm going to delete this snippet again and modify the affected doctests.

Do you agree with my conclusion and procedure?

A further doctest change is devoted to #28578.

[egourgoulhon]

comment:15

Replying to @DeRhamSource:

After a bit of thinking, I came to the conclusion that the code snippet should be obsolete.

Which code snippet are you talking about? All the code in this ticket branch?

[DeRhamSource]

comment:16

Replying to @egourgoulhon:

Replying to @DeRhamSource:

After a bit of thinking, I came to the conclusion that the code snippet should be obsolete.

Which code snippet are you talking about? All the code in this ticket branch?

No, only the last two/three commits. Namely the changes in set_restriction of mixed_form.py.

[egourgoulhon]

comment:17

Replying to @DeRhamSource:

After a bit of thinking, I came to the conclusion that the code snippet should be obsolete. (As a mathematician, I feel the urge to solve or fix a problem/error as soon as it occurs. Here: Doctest errors.) The thing is, due to our little discussion in #28519, the doctest must be flawed since mixed forms defined via preconstructed forms get altered.

So, I 'm going to delete this snippet again and modify the affected doctests.

Do you agree with my conclusion and procedure?

I am not sure to understand what you have in mind; I would say please go on and implement what you think is the best solution.

[sagetrac-git]

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

c104fc8

Scalar field zero treatment

[sagetrac-git]

Changed commit from 2865535 to c104fc8

[sagetrac-git]

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

efd8e03

Mixed form zero treatment

[sagetrac-git]

Changed commit from c104fc8 to efd8e03

[egourgoulhon]

comment:30

I just gave a quick look at the latest version. Looks good. I'll have a deeper look tomorrow.

[sagetrac-git]

Changed commit from efd8e03 to a91dba0

[sagetrac-git]

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

17e6ff6	Merge branch 'develop' into t/28562/better_zero_treatment_alternative
a91dba0	Zero if wedge result degree > dim

[egourgoulhon]

comment:32

I've finally gone through the ticket. The impression stated in comment:30 is confirmed: this is a nice improvement of the code! Thank you. I have made minor modifications, which have been pushed in the above branch. In particular

• the pyflake error reported by the patchbot is corrected
• t(0) has been changed to t.zero() and gl(1) to gl.one() (to skip one function call)
• some add/set_comp(basis) have been changed to add/set_comp(basis=basis)
• the treatment of zero has been added in FreeModuleTensor._sub_()

Do you agree with these changes?

---

New commits:

2783253

Minor changes in the zero and one treatment of tensor fields

[egourgoulhon]

Changed commit from a91dba0 to 2783253

[egourgoulhon]

Changed branch from u/gh-DeRhamSource/better_zero_treatment_alternative to public/manifolds/better_zero_treatment

[DeRhamSource]

comment:33

Yes, looks good! :)

[tscrim]

comment:34

I just have two trivial changes:

-The components w.r.t. basis e have been kept::
+The components with respect to basis ``e`` have been kept::

and in automorphismfield.py:

    def add_comp(self, basis=None):
        r"""

        Return the components of ``self`` w.r.t. a given module basis for
        assignment, keeping the components w.r.t. other bases.

        To delete the components w.r.t. other bases, use the method
        :meth:`set_comp` instead.

remove the blank line after r""" and same thing of spelling out w.r.t. (although I suspect this is done elsewhere, I think it is better to avoid abbreviations like this as this is a more "formal" document, but I don't care too much).

[egourgoulhon]

comment:35

On my side, one last remark: the names of the methods _add_comp_unsafe() and _set_comp_unsafe() give the impression that they are really dangerous methods that should be avoided, while they are key methods, doing most of the work of add_comp() and set_comp().
Since they are private methods, I don't think it is necessary to insist too much on their "unsafe" aspect, which is truly unsafe only when invoked on the special elements zero and one. Wouldn't something like _add_comp_nocheck() and _set_comp_nocheck() be better names? Travis, do you have an opinion on that?

[tscrim]

comment:36

This naming convention matches what is done for vectors (and maybe matrices?) in Sage. Although in that case, if you mess with things in the wrong way, you will crash Sage. However, given the nomenclature of other methods, I think *_unsafe is the way to go.

[egourgoulhon]

comment:37

Replying to @tscrim:

This naming convention matches what is done for vectors (and maybe matrices?) in Sage.

Ah yes! (grep -r "_unsafe" in src/sage returns 872 entries; I was not aware of that). So OK for *_unsafe then...

[sagetrac-git]

Changed commit from 2783253 to f2928f8

[sagetrac-git]

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

f2928f8

Blankline removed

[DeRhamSource]

comment:39

Replying to @tscrim:

I just have two trivial changes:

-The components w.r.t. basis e have been kept::
+The components with respect to basis ``e`` have been kept::

Since this issue is not just devoted to this ticket, I vote for a new ticket on that.

and in automorphismfield.py:

    def add_comp(self, basis=None):
        r"""

        Return the components of ``self`` w.r.t. a given module basis for
        assignment, keeping the components w.r.t. other bases.

        To delete the components w.r.t. other bases, use the method
        :meth:`set_comp` instead.

remove the blank line after r""" and same thing of spelling out w.r.t. (although I suspect this is done elsewhere, I think it is better to avoid abbreviations like this as this is a more "formal" document, but I don't care too much).

Done.

[egourgoulhon]

comment:40

Replying to @DeRhamSource:

Replying to @tscrim:

I just have two trivial changes:

-The components w.r.t. basis e have been kept::
+The components with respect to basis ``e`` have been kept::

Since this issue is not just devoted to this ticket, I vote for a new ticket on that.

Indeed. I am afraid I am responsible for most of these w.r.t. ;-)

[egourgoulhon]

comment:41

Thanks for your work on this!

[egourgoulhon]

Reviewer: Eric Gourgoulhon, Travis Scrimshaw

[vbraun]

Changed branch from public/manifolds/better_zero_treatment to f2928f8