Tensor Fields: set_restriction Behaviour

[gh-DeRhamSource]

The following code leads to an error:

sage: M = Manifold(2, 'M')
sage: X.<x,y> = M.chart()
sage: eX = X.frame()
sage: a = M.one_form()
sage: b = M.one_form()
sage: a[eX,:] = x*y, x+y
sage: b.set_restriction(a)
sage: b.display()

ValueError   Traceback (most recent call last)
...

ValueError: no basis could be found for computing the components in the Coordinate frame (M, (d/dx,d/dy))

Mathematically, this should not happen.

A method copy_from is added, and in case of the same domain, the copy_from method is invoked, when set_restriction is used.

This ticket is part of the metaticket #28519.

[gh-DeRhamSource]

New commits:

​e30ec5b

'set_restriction' modified and 'copy_from' method added

[git]

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

​88c31f9

Clear components when 'copy_from' is invoked

[egourgoulhon]

Thanks for opening the ticket and implementing the fix (the introduction of copy_from seems a good idea). However I don't think that you should add the optional argument del_restrictions in TensorField._del_derived. At first glance, this makes this method on the same footing than TensorFieldParal._del_derived, which has such an argument. However, TensorField and TensorFieldParal are not symmetrical as far as restrictions are concerned (see Sec. 3.5 of ​these notes):

• for a TensorFieldParal, the primary data are the components w.r.t. to vector frames on its domain, stored in the dict _components; the restrictions are derived quantities, which can be reconstructed at any time from _components
• for a TensorField, the primary data are the restrictions to subdomains; they are not derived quantities; deleting them in _del_derived() would erase essential piece of information on the tensor field

[gh-DeRhamSource]

Actually, I'm aware of that and definitely see your point. Though, as one can see in add_expr_by_continuation (and now copy_from), this behaviour is wanted at some point, even for TensorField objects. For that reason, I put del_restrictions=False as default, so that the previous code is not affected at all.

If you don't want to have this snippet in _del_derived, where it would make sense in my opinion, I'd suggest a new method _del_restrictions for it is used quite often. This simplifies future modifications on this matter (if needed).

What do you say?

[gh-DeRhamSource]

Ah! For add_expr_by_continuation you want exactly the behaviour you explained, of course. Sorry! However, imho the modifications above make things easier and the code more elegant and consistent.

[git]

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

​672d268

Reorganization of restriction deletion

[gh-DeRhamSource]

Is this better?

[egourgoulhon]

Replying to gh-DeRhamSource:

Is this better?

Much better! ;-)

A question though: what's the point of

    TensorField.copy_from(self, other)

in TensorFieldParal.copy_from() ? Actually, the action of TensorField.copy_from amounts to copying the restrictions, but the restrictions are deleted by the subsequent call:

    FreeModuleTensor.copy_from(self, other)

due to

    self._del_derived()

in FreeModuleTensor.copy_from().

In view of this, I would simply delete TensorFieldParal.copy_from(), so that copy_from() in TensorFieldParal is inherited from FreeModuleTensor.

[gh-DeRhamSource]

Replying to egourgoulhon:

Much better! ;-)

A question though: what's the point of

    TensorField.copy_from(self, other)

in TensorFieldParal.copy_from() ? Actually, the action of TensorField.copy_from amounts to copying the restrictions, but the restrictions are deleted by the subsequent call:

    FreeModuleTensor.copy_from(self, other)

due to

    self._del_derived()

in FreeModuleTensor.copy_from().

Ah! Yes! The order should be switched then, right?

A copy of the restrictions even for TensorFieldParal would be convenient, wouldn't it?

[egourgoulhon]

Replying to gh-DeRhamSource:

Ah! Yes! The order should be switched then, right?

Yes if you would like to maintain TensorFieldParal.copy_from() with the copies of the restrictions.

A copy of the restrictions even for TensorFieldParal would be convenient, wouldn't it?

I am not so sure: for TensorFieldParal, restrictions are derived quantities, which can be easily reconstructed at any time if necessary. Not copying them makes copy_from() faster. If we have a.copy_from(b) with restrictions of b having been constructed for whatever reasons, it could be that in the future use of a, no restriction will be invoked. So I would vote for removing copy_from() from TensorFieldParal class, keeping it only in FreeModuleTensor. This makes the code shorter and clearer: copy_from() does not copy the derived quantities, whatever their nature.

[gh-DeRhamSource]

Okay, that sounds reasonable. I will work on this tomorrow.

By the way, characteristic classes are almost finished. They come with a very rudimentary version of bundle connections. But I have already plans for full bundle connection support. Though, this has to wait since my master thesis deadline runs out soon.

Best wishes, Michael

[git]

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

​c790d13

'copy_from' removed

[gh-DeRhamSource]

Done.

[egourgoulhon]

Replying to gh-DeRhamSource:

Done.

Thanks.

[chapoton]

author name missing

[vbraun]

Merge conflict

[egourgoulhon]

Replying to vbraun:

Merge conflict

I guess this is a conflict with one of the recently merged manifold tickets, #28600 or #28159 (more probably the latter). The easiest way to solve it would be to wait for the 9.0.beta3 release...

[gh-DeRhamSource]

No, the affected files are untouched in #28600 and #28159. I rather think it's connected to #28562?

Which code lines cause a merge conflict anyway?

[egourgoulhon]

Replying to gh-DeRhamSource:

No, the affected files are untouched in #28600 and #28159. I rather think it's connected to #28562?

I don't think so, since #28562 is not merged yet. Note that #28159 is touching tensorfield.py, as the current ticket.

Which code lines cause a merge conflict anyway?

It will be easy to see them when the develop branch is released (as 9.0.beta3).

[gh-DeRhamSource]

Well, okay. But this is just one line with a typo in a very different line. However, I think you're right.

[git]

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

​4c1de40

Merge branch 'develop' into t/28628/tensor_fields__set_restriction_behaviour

[gh-DeRhamSource]

Merge still fails. Is it possible to see the conflict files/lines?

[egourgoulhon]

The merge conflict with Sage 9.0.beta3 is solved in the above branch. I have also added the copy of the attribute _is_zero in copy_from(), as well as some autorship lines. Do you agree?

---

New commits:

​c3666e8	Resolve merge conflict of #28628 in Sage 9.0.beta3
​9299f89	Copy _is_zero in copy_from

[gh-DeRhamSource]

Yes! Thank you very much! :)

[egourgoulhon]

Replying to gh-DeRhamSource:

Merge still fails. Is it possible to see the conflict files/lines?

Sorry I haven't seen your message before pushing the new branch; it seems we were working on solving the conflict at the same time ;-).

The general method I am using to see the conflicting files/lines is the following one:

• start from a clean develop branch (i.e. 9.0.beta3 in the present case)
• do

  git checkout -b set_restriction-28628
  git pull trac u/gh-DeRhamSource/tensor_fields__set_restriction_behaviour
  

This generates an error message, stating that there is a conflict in tensorfield.py. Edit the latter to fix the conflict and do git add tensorfield.py, then git commit, etc.

[gh-DeRhamSource]

That's a good hint for the future. I just thought, you were fast! :D

However, you solved the merge conflict and I'd say, we take your branch, now.

[egourgoulhon]

Replying to gh-DeRhamSource:

That's a good hint for the future.

It seems there is a merge conflict between 9.0.beta3 and #28564; so if you want to practice...

[egourgoulhon]

The last patchbot is full green.