Opened 2 years ago

Closed 2 years ago

#30209 closed enhancement (fixed)

Action for Bundle Connections

Reported by: Michael Jung Owned by:
Priority: major Milestone: sage-9.2
Component: manifolds Keywords:
Cc: Eric Gourgoulhon, Travis Scrimshaw Merged in:
Authors: Michael Jung Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: 7ae2df9 (Commits, GitHub, GitLab) Commit: 7ae2df97d0c1d57d8beab4932d5b22ec827f1079
Dependencies: #30208, #30228 Stopgaps:

Status badges

Description (last modified by Michael Jung)

In this ticket, we establish the action of a bundle connection. We implement the component-wise formula based on the connection form.

See Wikipedia for details.

Change History (24)

comment:1 Changed 2 years ago by Michael Jung

Description: modified (diff)

comment:2 Changed 2 years ago by Michael Jung

Branch: u/gh-mjungmath/bundle_connection_change_of_frame

comment:3 Changed 2 years ago by Michael Jung

Commit: 56996fd684b86009a72ec7aca587ad3e41c7020d
Description: modified (diff)
Summary: Change of Frame Formula for Bundle ConnectionsAction for Bundle Connections

Last 10 new commits:

f0c6ee8Trac #30191: minor fix
f1221daTrac #30191: minor fix
a042313Trac# 30191: wrong indentation reverted
4cc92e9Trac #30191: comp check unified to if comp
f98a47bTrac #30191: unused ScalarField import removed
9c5a25bTrac #30208: assigment for bundle connections
fa87f4bTrac #30208: documentation improved
b7a6a5fMerge branch 'develop' into bundle_connection_change_of_frame
0c28d89Trac #30209: forms from scratch
56996fdTrac #30209: bundle connection action

comment:4 Changed 2 years ago by Michael Jung

Dependencies: #30208#30208, #30228

comment:5 Changed 2 years ago by Michael Jung

Dependencies: #30208, #30228#30208, #30228, #30239

comment:6 Changed 2 years ago by git

Commit: 56996fd684b86009a72ec7aca587ad3e41c7020d0e3cba0fc21c4da924bda56aa559ec313eef8376

Branch pushed to git repo; I updated commit sha1. Last 10 new commits:

ef8ac4bTrac #30209: display method + merge
51ba4e3Trac #30239: comp zero check for __call__
bd1f299Trac #30209: Merge branch 't/30239/tensorfield___call___alters_zero' into bundle_connection_change_of_frame
e7fedb7Trac #30209: display method finished
b03baa1Trac #30209: structure formula fixed
bb476e2Trac #30208: minor fix
a77aebcTrac #30208: copy_from reverted to recover old behavior
934a439documentation slightly improved
574445dTrac #30209: Merge branch 'bundle_connection_extension' into bundle_connection_change_of_frame
0e3cba0Trac #30209: documentation finished + display only_nonzero fixed

comment:7 Changed 2 years ago by Michael Jung

Status: newneeds_review

comment:8 Changed 2 years ago by git

Commit: 0e3cba0fc21c4da924bda56aa559ec313eef83762dd78aacdcff90f43f6e0618dbc10b554e7269d7

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

2dd78aaTrac #30209: documentation improved

comment:9 Changed 2 years ago by git

Commit: 2dd78aacdcff90f43f6e0618dbc10b554e7269d7e5a3ac4ebaf740396d1bc32bd7d012d560215318

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

e5a3ac4Trac #30209: documentation imrpoved

comment:10 Changed 2 years ago by Michael Jung

I don't think the depencency of #30239 is necessary anymore. Anyway, I'll leave it this way. Removing it again would be too complicated by now...

comment:11 Changed 2 years ago by Michael Jung

The pycodestyle error has been fixed in #30239.

comment:12 Changed 2 years ago by git

Commit: e5a3ac4ebaf740396d1bc32bd7d012d560215318933c14aa016e1cc23be47caf845199c7e1cbf52e

Branch pushed to git repo; I updated commit sha1. Last 10 new commits:

504e84eTrac #30239: Merge branch 't/30266/immutability_for_scalar_fields' into t/30239/tensorfield___call___alters_zero
d58d291Trac #30266: hash function condition + treatment of restrictions
061b89fTrac #30266: minor doctest improvement
5c7fd6eTrac #30266: immutability of restrictions + hash function improved
7ba0865Trac #30266: merge
0ec99d4Trac #30266: check by name
87fb411Trac #30266: ValueError replaced by AssertionError
43ec497Trac #30239: referenced before assignment error fixed
135ffd5Trac #30209: Merge branch 't/30239/tensorfield___call___alters_zero' into bundle_connection_change_of_frame
933c14aTrac #30209: Merge branch 'develop' into bundle_connection_change_of_frame

comment:13 Changed 2 years ago by git

Commit: 933c14aa016e1cc23be47caf845199c7e1cbf52e21f2bd5a70a29dae8cd699ba03f8eb05870bdd7e

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

d3c8cabTrac #30208: random doctest removed
5cee80cTrac #30209: Merge branch 'bundle_connection_extension' into bundle_connection_action
21f2bd5Trac #30209: bundle connections acting on sections + vector fields

comment:14 Changed 2 years ago by Michael Jung

Dependencies: #30208, #30228, #30239#30208, #30228

I have removed the dependency of #30239.

comment:15 Changed 2 years ago by Michael Jung

Green patchbot.

comment:16 Changed 2 years ago by Travis Scrimshaw

I don't like doctests marked as random that are not random:

sage: nab.connection_forms() # random

Just go through the different values and get the output explicitly (and maybe check the len). This will be more robust and actually catch errors.

Your error messages in display don't need to be so excited (just remove the ! from the end).

While it is good to keep encapsulation, just be aware that changes like this:

-                    rk = vb._rank
+                    rk = vb.rank()

mean extra Python function calls, which are relatively expensive compared to attribute lookup. However, I have no objections to these changes as they seem like they would not have any significant impact on computation times.

comment:17 in reply to:  16 Changed 2 years ago by Michael Jung

Replying to tscrim:

I don't like doctests marked as random that are not random:

sage: nab.connection_forms() # random

Just go through the different values and get the output explicitly (and maybe check the len). This will be more robust and actually catch errors.

Ah, you already mentioned that in some ticket. Sure, I will take care of that.

Your error messages in display don't need to be so excited (just remove the ! from the end).

:(

While it is good to keep encapsulation, just be aware that changes like this:

-                    rk = vb._rank
+                    rk = vb.rank()

mean extra Python function calls, which are relatively expensive compared to attribute lookup. However, I have no objections to these changes as they seem like they would not have any significant impact on computation times.

Ah, this is good to know! I'll change it back.

Tomorrow...

comment:18 Changed 2 years ago by git

Commit: 21f2bd5a70a29dae8cd699ba03f8eb05870bdd7e7ae2df97d0c1d57d8beab4932d5b22ec827f1079

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

ec05c7fTrac #30209: Merge branch 't/30208/d3c8cab917f04ebc9951d41a0cc8dcf4c66392a6' into new_bundle_connection_action
7ae2df9Trac #30209: action of bundle connections + display method

comment:19 Changed 2 years ago by Michael Jung

I see. Something went wrong during the rebase. Please double check in comparison with #30208.

comment:20 Changed 2 years ago by Michael Jung

I've forced a new push btw. Like I said, please take a thorough look.

comment:21 Changed 2 years ago by Michael Jung

Compared with #30208 once again. Looks good, like intended. Patchbot is green again.

comment:22 Changed 2 years ago by Travis Scrimshaw

Reviewers: Travis Scrimshaw
Status: needs_reviewpositive_review

Thank you. LGTM.

comment:23 Changed 2 years ago by Michael Jung

Thank you for the review.

comment:24 Changed 2 years ago by Volker Braun

Branch: u/gh-mjungmath/bundle_connection_change_of_frame7ae2df97d0c1d57d8beab4932d5b22ec827f1079
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.