Opened 5 years ago
Closed 5 years ago
#25417 closed enhancement (fixed)
Better restrictions on manifolds
Reported by:  ghFlorentinJ  Owned by:  

Priority:  major  Milestone:  sage8.3 
Component:  geometry  Keywords:  restriction, extension, manifold, subdomain, graph 
Cc:  Eric Gourgoulhon  Merged in:  
Authors:  Florentin Jaffredo  Reviewers:  Eric Gourgoulhon 
Report Upstream:  N/A  Work issues:  
Branch:  cbbbf47 (Commits, GitHub, GitLab)  Commit:  cbbbf47311d1043297dc5d0138f2285c0ce852ef 
Dependencies:  Stopgaps: 
Description
This ticket implements two new attributes of tensor fields on manifolds used to improve the restriction operation, by using rudimentary graph exploration to check for existing restriction and extensions.
Previously, defining the restriction of a tensor field on a subset V of a subset U before the restriction on the subset U would not register the first tensor as a restriction of the second, resulting in the creation of two python objects, despite being mathematically identical.
An example of code reproducing this behavior can be seen on this notebook. With this ticket, the order of creation no longer matters.
This is part of the SageManifolds project; see the metaticket #18528 for an overview.
Change History (8)
comment:1 Changed 5 years ago by
Commit:  5bd63520271cfa2b13cc2c65a4f5c948c00c59a2 → 9700c8b61da4af896510722f07cc47896918fe8b 

comment:2 Changed 5 years ago by
Status:  new → needs_review 

comment:3 Changed 5 years ago by
Reviewers:  → Eric Gourgoulhon 

LGTM.
Simply, could you add some Python comments in lines 426427 of tensorfield.py
regarding the meaning of the new attributes _extensions_graph
and _restrictions_graph
? This would be helpfull, especially since, technically speaking, these attributes are not graphs, but dictionaries.
comment:4 Changed 5 years ago by
Commit:  9700c8b61da4af896510722f07cc47896918fe8b → db9cd160c2a86cdf91968625c0a9a0677b4f1f56 

Branch pushed to git repo; I updated commit sha1. New commits:
db9cd16  Added some comments to explain the purpose of new attributes

comment:5 Changed 5 years ago by
Status:  needs_review → positive_review 

Thanks for the changes. LGTM. I think the failures in the plugins startup_time
and startup_modules
reported by the patchbot are irrelevant as far as this ticket is concerned.
comment:6 Changed 5 years ago by
Commit:  db9cd160c2a86cdf91968625c0a9a0677b4f1f56 → cbbbf47311d1043297dc5d0138f2285c0ce852ef 

Status:  positive_review → needs_review 
Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:
cbbbf47  missing 's' in a undoctested piece of code

comment:7 Changed 5 years ago by
Status:  needs_review → positive_review 

OK, so everything is green now, including the patchbot.
comment:8 Changed 5 years ago by
Branch:  public/manifolds/better_restrictions → cbbbf47311d1043297dc5d0138f2285c0ce852ef 

Resolution:  → fixed 
Status:  positive_review → closed 
Branch pushed to git repo; I updated commit sha1. New commits:
Merge branch with 8.3.beta2