Opened 2 years ago
Last modified 2 years ago
#28628 closed defect
Tensor Fields: set_restriction Behaviour — at Version 7
Reported by:  ghDeRhamSource  Owned by:  

Priority:  major  Milestone:  sage9.0 
Component:  geometry  Keywords:  manifolds, tensor 
Cc:  egourgoulhon  Merged in:  
Authors:  Reviewers:  
Report Upstream:  N/A  Work issues:  
Branch:  u/ghDeRhamSource/tensor_fields__set_restriction_behaviour (Commits, GitHub, GitLab)  Commit:  88c31f99d477220b17dea0795deea481b9660f49 
Dependencies:  Stopgaps: 
Description (last modified by )
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.
Change History (7)
comment:1 Changed 2 years ago by
 Branch set to u/ghDeRhamSource/tensor_fields__set_restriction_behaviour
comment:2 Changed 2 years ago by
 Cc egourgoulhon added
 Commit set to e30ec5b88f8c3144c08e731d85c4c79e58595cc4
 Description modified (diff)
 Keywords manifolds added
 Status changed from new to needs_review
 Type changed from PLEASE CHANGE to defect
comment:3 Changed 2 years ago by
 Commit changed from e30ec5b88f8c3144c08e731d85c4c79e58595cc4 to 88c31f99d477220b17dea0795deea481b9660f49
Branch pushed to git repo; I updated commit sha1. New commits:
88c31f9  Clear components when 'copy_from' is invoked

comment:4 Changed 2 years ago by
 Component changed from PLEASE CHANGE to geometry
 Status changed from needs_review to needs_work
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
comment:5 Changed 2 years ago by
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?
comment:6 Changed 2 years ago by
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.
comment:7 Changed 2 years ago by
 Description modified (diff)
New commits:
'set_restriction' modified and 'copy_from' method added