Opened 3 months ago

Closed 3 months ago

#30209 closed enhancement (fixed)

Action for Bundle Connections

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

Description (last modified by gh-mjungmath)

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 3 months ago by gh-mjungmath

  • Description modified (diff)

comment:2 Changed 3 months ago by gh-mjungmath

  • Branch set to u/gh-mjungmath/bundle_connection_change_of_frame

comment:3 Changed 3 months ago by gh-mjungmath

  • Commit set to 56996fd684b86009a72ec7aca587ad3e41c7020d
  • Description modified (diff)
  • Summary changed from Change of Frame Formula for Bundle Connections to Action 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 3 months ago by gh-mjungmath

  • Dependencies changed from #30208 to #30208, #30228

comment:5 Changed 3 months ago by gh-mjungmath

  • Dependencies changed from #30208, #30228 to #30208, #30228, #30239

comment:6 Changed 3 months ago by git

  • Commit changed from 56996fd684b86009a72ec7aca587ad3e41c7020d to 0e3cba0fc21c4da924bda56aa559ec313eef8376

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 3 months ago by gh-mjungmath

  • Status changed from new to needs_review

comment:8 Changed 3 months ago by git

  • Commit changed from 0e3cba0fc21c4da924bda56aa559ec313eef8376 to 2dd78aacdcff90f43f6e0618dbc10b554e7269d7

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

2dd78aaTrac #30209: documentation improved

comment:9 Changed 3 months ago by git

  • Commit changed from 2dd78aacdcff90f43f6e0618dbc10b554e7269d7 to e5a3ac4ebaf740396d1bc32bd7d012d560215318

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

e5a3ac4Trac #30209: documentation imrpoved

comment:10 Changed 3 months ago by gh-mjungmath

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 3 months ago by gh-mjungmath

The pycodestyle error has been fixed in #30239.

comment:12 Changed 3 months ago by git

  • Commit changed from e5a3ac4ebaf740396d1bc32bd7d012d560215318 to 933c14aa016e1cc23be47caf845199c7e1cbf52e

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 3 months ago by git

  • Commit changed from 933c14aa016e1cc23be47caf845199c7e1cbf52e to 21f2bd5a70a29dae8cd699ba03f8eb05870bdd7e

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 3 months ago by gh-mjungmath

  • Dependencies changed from #30208, #30228, #30239 to #30208, #30228

I have removed the dependency of #30239.

comment:15 Changed 3 months ago by gh-mjungmath

Green patchbot.

comment:16 follow-up: Changed 3 months ago by 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.

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 3 months ago by gh-mjungmath

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 3 months ago by git

  • Commit changed from 21f2bd5a70a29dae8cd699ba03f8eb05870bdd7e to 7ae2df97d0c1d57d8beab4932d5b22ec827f1079

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 3 months ago by gh-mjungmath

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

comment:20 Changed 3 months ago by gh-mjungmath

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

comment:21 Changed 3 months ago by gh-mjungmath

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

comment:22 Changed 3 months ago by tscrim

  • Reviewers set to Travis Scrimshaw
  • Status changed from needs_review to positive_review

Thank you. LGTM.

comment:23 Changed 3 months ago by gh-mjungmath

Thank you for the review.

comment:24 Changed 3 months ago by vbraun

  • Branch changed from u/gh-mjungmath/bundle_connection_change_of_frame to 7ae2df97d0c1d57d8beab4932d5b22ec827f1079
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.