Opened 4 years ago
Closed 4 years ago
#25417 closed enhancement (fixed)
Better restrictions on manifolds
Reported by: | gh-FlorentinJ | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-8.3 |
Component: | geometry | Keywords: | restriction, extension, manifold, subdomain, graph |
Cc: | egourgoulhon | 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 4 years ago by
- Commit changed from 5bd63520271cfa2b13cc2c65a4f5c948c00c59a2 to 9700c8b61da4af896510722f07cc47896918fe8b
comment:2 Changed 4 years ago by
- Status changed from new to needs_review
comment:3 Changed 4 years ago by
- Reviewers set to Eric Gourgoulhon
LGTM.
Simply, could you add some Python comments in lines 426-427 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 4 years ago by
- Commit changed from 9700c8b61da4af896510722f07cc47896918fe8b to 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 4 years ago by
- Status changed from needs_review to 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 4 years ago by
- Commit changed from db9cd160c2a86cdf91968625c0a9a0677b4f1f56 to cbbbf47311d1043297dc5d0138f2285c0ce852ef
- Status changed from positive_review to 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 un-doctested piece of code
|
comment:7 Changed 4 years ago by
- Status changed from needs_review to positive_review
OK, so everything is green now, including the patchbot.
comment:8 Changed 4 years ago by
- Branch changed from public/manifolds/better_restrictions to cbbbf47311d1043297dc5d0138f2285c0ce852ef
- Resolution set to fixed
- Status changed from positive_review to closed
Branch pushed to git repo; I updated commit sha1. New commits:
Merge branch with 8.3.beta2