Opened 5 years ago

Closed 5 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: 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:

Status badges

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 git

Commit: 5bd63520271cfa2b13cc2c65a4f5c948c00c59a29700c8b61da4af896510722f07cc47896918fe8b

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

9700c8bMerge branch with 8.3.beta2

comment:2 Changed 5 years ago by gh-FlorentinJ

Status: newneeds_review

comment:3 Changed 5 years ago by Eric Gourgoulhon

Reviewers: 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 5 years ago by git

Commit: 9700c8b61da4af896510722f07cc47896918fe8bdb9cd160c2a86cdf91968625c0a9a0677b4f1f56

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

db9cd16Added some comments to explain the purpose of new attributes

comment:5 Changed 5 years ago by Eric Gourgoulhon

Status: needs_reviewpositive_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 git

Commit: db9cd160c2a86cdf91968625c0a9a0677b4f1f56cbbbf47311d1043297dc5d0138f2285c0ce852ef
Status: positive_reviewneeds_review

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

cbbbf47missing 's' in a un-doctested piece of code

comment:7 Changed 5 years ago by Eric Gourgoulhon

Status: needs_reviewpositive_review

OK, so everything is green now, including the patchbot.

comment:8 Changed 5 years ago by Volker Braun

Branch: public/manifolds/better_restrictionscbbbf47311d1043297dc5d0138f2285c0ce852ef
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.