Opened 2 years ago
Closed 13 months ago
#30272 closed enhancement (fixed)
Mixed Forms: set_comp, comp
Reported by: | gh-mjungmath | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-9.4 |
Component: | manifolds | Keywords: | |
Cc: | egourgoulhon, tscrim, mkoeppe | Merged in: | |
Authors: | Michael Jung | Reviewers: | Eric Gourgoulhon |
Report Upstream: | N/A | Work issues: | |
Branch: | 91fc96f (Commits, GitHub, GitLab) | Commit: | 91fc96fed615f6070a729c143703907dc227819e |
Dependencies: | #31654, #30473 | Stopgaps: |
Description (last modified by )
In the upcoming changes we want to allow mixed differential forms, among other things, being made immutable (see #30181, #30261).
However, the current implementation would not support that behavior properly, namely:
sage: M = Manifold(2, 'M') sage: A = M.mixed_form() sage: a = M.diff_form(2) sage: A[2] = a sage: A[2] is a True
If A
would be set immutable and a
is not, this would contradict the behavior of an immutable element. Even if a
would automatically be set immutable as soon as A
had been set immutable, this might happen not on the behalf of the user.
I'd propose a similar approach as I had done in #30208 for bundle connections, and as it is already known for tensor fields, namely by introducing methods set_comp
and comp
(add_comp
would not be necessary). Then each instance representing a homogeneous component is bound only to the mixed differential form.
As always, suggestions and opinions are welcome.
Attachments (1)
Change History (34)
comment:1 Changed 2 years ago by
- Cc mkoeppe added
comment:2 Changed 2 years ago by
- Summary changed from Mixed Forms: set_comp, add_comp, comp to Mixed Forms: add_comp, comp
comment:3 Changed 2 years ago by
- Summary changed from Mixed Forms: add_comp, comp to Mixed Forms: set_comp, comp
comment:4 Changed 2 years ago by
- Description modified (diff)
comment:5 Changed 2 years ago by
- Milestone changed from sage-9.2 to sage-9.3
comment:6 Changed 18 months ago by
- Milestone changed from sage-9.3 to sage-9.4
comment:7 Changed 16 months ago by
- Dependencies set to #31653
comment:8 Changed 16 months ago by
- Dependencies changed from #31653 to #31654
comment:9 Changed 16 months ago by
- Branch set to u/gh-mjungmath/mixed_forms__set_comp__comp
comment:10 Changed 16 months ago by
- Commit set to 69f7bb51b0bd5d0cfe3db416e4e15b3bedc9997a
Branch pushed to git repo; I updated commit sha1. New commits:
69f7bb5 | Trac #30272: new standard name for homogeneous components (without tests)
|
comment:11 Changed 16 months ago by
First approach.
What I did:
- homogeneous components are bound to mixed form and a priori detached from other instances; in particular
sage: A[0] = f sage: A[0] is f
yields
False
now.
- components are only initialized on demand (not on initialization of the mixed form)
- by default, homogeneous components are named according to the mixed form's name:
A = A_0, A_1, ...
set_comp
method similar to tensor fields
I tried to maintain as much backwards compatibility as possible. For example, even if copies now, the names are still applied if A[:] = [f, omega, eta]
syntax is used. But the long-term idea is to favor set_comp(i)[:] = ...
syntax instead of omega[:] = ...
plus A[i] = omega
syntax.
Comments are appreciated.
comment:12 Changed 16 months ago by
- Commit changed from 69f7bb51b0bd5d0cfe3db416e4e15b3bedc9997a to 27aa360aebdcdd632fa62d6d3161d475f3954221
Branch pushed to git repo; I updated commit sha1. New commits:
27aa360 | Trac #30272: doctests added
|
comment:13 Changed 16 months ago by
- Status changed from new to needs_review
Here we go, that should be it. Please take a close look whether this is (at least) halfway consistent and transparent to the user.
In a next step, mixed forms will be equipped with an immutability switch.
comment:14 Changed 16 months ago by
If desired so, I will squash the commits for final merge.
comment:15 Changed 16 months ago by
comment:16 Changed 16 months ago by
- Reviewers set to Eric Gourgoulhon
- Status changed from needs_review to positive_review
LGTM.
comment:17 Changed 14 months ago by
- Status changed from positive_review to needs_work
Merge conflict
comment:18 Changed 14 months ago by
- Commit changed from 27aa360aebdcdd632fa62d6d3161d475f3954221 to 89dc5520f76fc7ef937c073e5a799925491a88ea
Branch pushed to git repo; I updated commit sha1. New commits:
89dc552 | Trac #30272: merged
|
comment:19 Changed 14 months ago by
- Status changed from needs_work to needs_review
comment:20 follow-up: ↓ 22 Changed 14 months ago by
When clicking on the new branch, one gets a huge diff, which seems to be the whole 9.4.beta3 w.r.t 9.4.beta2. Wouldn't it be better to merge the previous ticket branch into a clean 9.4.beta3?
Changed 14 months ago by
comment:21 Changed 14 months ago by
comment:22 in reply to: ↑ 20 ; follow-up: ↓ 23 Changed 14 months ago by
Replying to egourgoulhon:
When clicking on the new branch, one gets a huge diff, which seems to be the whole 9.4.beta3 w.r.t 9.4.beta2. Wouldn't it be better to merge the previous ticket branch into a clean 9.4.beta3?
That sometimes happens with the trac git helper. You don't have to worry too much about it as long as it pulls okay (a la comment:21).
comment:23 in reply to: ↑ 22 Changed 14 months ago by
Replying to tscrim:
Replying to egourgoulhon:
When clicking on the new branch, one gets a huge diff, which seems to be the whole 9.4.beta3 w.r.t 9.4.beta2. Wouldn't it be better to merge the previous ticket branch into a clean 9.4.beta3?
That sometimes happens with the trac git helper. You don't have to worry too much about it as long as it pulls okay (a la comment:21).
Indeed, this seems erratic: it looks good now, while the branch has not been changed.
comment:24 Changed 14 months ago by
- Status changed from needs_review to positive_review
comment:25 Changed 14 months ago by
- Status changed from positive_review to needs_work
[release@zen Sage]$ sage -t --long --warn-long 49.3 --random-seed=0 src/sage/manifolds/differentiable/mixed_form.py # 4 doctests failed Running doctests with ID 2021-06-29-21-56-39-c62006fe. Git branch: develop Using --optional=build,dochtml,fedora,pip,sage,sage_spkg Doctesting 1 file. sage -t --long --warn-long 49.3 --random-seed=0 src/sage/manifolds/differentiable/mixed_form.py ********************************************************************** File "src/sage/manifolds/differentiable/mixed_form.py", line 90, in sage.manifolds.differentiable.mixed_form.MixedForm Failed example: A.display_expansion() # display expansion in basis Expected: A = [x] + [x*y dx] + [x*y^2 dx/\dy] Got: A = x + x*y dx + x*y^2 dx/\dy ********************************************************************** File "src/sage/manifolds/differentiable/mixed_form.py", line 113, in sage.manifolds.differentiable.mixed_form.MixedForm Failed example: B.display_expansion() Expected: B = [x] + [x*y dx] + [x*y^2 dx/\dy] Got: B = x + x*y dx + x*y^2 dx/\dy ********************************************************************** [...]
comment:26 Changed 14 months ago by
Ah damn, I mis-resolved the merge conflict.
Let's wait for #30473 anyway.
comment:27 Changed 14 months ago by
- Dependencies changed from #31654 to #31654, #30473
comment:28 Changed 13 months ago by
- Commit changed from 89dc5520f76fc7ef937c073e5a799925491a88ea to f1e7ef884b03cf3ad6a7fe09b0b8892acc818063
Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:
da8893f | Use sage.typeset.unicode_characters in TensorProductFunctor and SignedTensorProductFunctor
|
2a23cb5 | Unicode symbol 2202 (partial) for the text display of coordinate frames
|
5d096f1 | f-string for unicode_symbol in TensorProductFunctor and SignedTensorProductFunctor
|
76c2fd5 | Use Unicode symbol for the Riemann sphere example
|
5167e6c | Use Unicode symbol for default text display of RealLine
|
332410b | Use unicode_otimes in TensorProductFunctor and SignedTensorProductFunctor
|
f5d15d2 | Merge branch 'public/manifolds/unicode_art' of git://trac.sagemath.org/sage into Sage 9.4.beta4.
|
d87d09b | #30473: fix doctest error in DiffMap.pullback
|
d62adad | merge unicode
|
f1e7ef8 | Trac #30272: remove set_name_comp + fix doctest
|
comment:29 Changed 13 months ago by
- Status changed from needs_work to needs_review
comment:30 Changed 13 months ago by
- Commit changed from f1e7ef884b03cf3ad6a7fe09b0b8892acc818063 to 91fc96fed615f6070a729c143703907dc227819e
Branch pushed to git repo; I updated commit sha1. New commits:
91fc96f | Trac #30272: minor doctest fix
|
comment:31 Changed 13 months ago by
Ready for review again.
comment:32 Changed 13 months ago by
- Status changed from needs_review to positive_review
The patchbot reports doctest failures that all pertain to the dependency #30473. They are all fixed in the latest version of that ticket. But I don't think it necessary to merge it here to move on.
comment:33 Changed 13 months ago by
- Branch changed from u/gh-mjungmath/mixed_forms__set_comp__comp to 91fc96fed615f6070a729c143703907dc227819e
- Resolution set to fixed
- Status changed from positive_review to closed
Setting new milestone based on a cursory review of ticket status, priority, and last modification date.