Opened 2 years ago

Closed 2 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) 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 2 years ago by git

  • Commit changed from 5bd63520271cfa2b13cc2c65a4f5c948c00c59a2 to 9700c8b61da4af896510722f07cc47896918fe8b

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

​9700c8bMerge branch with 8.3.beta2

comment:2 Changed 2 years ago by gh-FlorentinJ

  • Status changed from new to needs_review

comment:3 Changed 2 years ago by egourgoulhon

  • 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 2 years ago by git

  • Commit changed from 9700c8b61da4af896510722f07cc47896918fe8b to db9cd160c2a86cdf91968625c0a9a0677b4f1f56

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

​db9cd16Added some comments to explain the purpose of new attributes

comment:5 Changed 2 years ago by egourgoulhon

  • 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 2 years ago by git

  • 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:

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

comment:7 Changed 2 years ago by egourgoulhon

  • Status changed from needs_review to positive_review

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

comment:8 Changed 2 years ago by vbraun

  • Branch changed from public/manifolds/better_restrictions to cbbbf47311d1043297dc5d0138f2285c0ce852ef
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.