Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Branch reduce propertyaccess wrap #3301

Open
wants to merge 36 commits into
base: main
Choose a base branch
from
Open

Branch reduce propertyaccess wrap #3301

wants to merge 36 commits into from

Conversation

aymanhab
Copy link
Member

@aymanhab aymanhab commented Oct 17, 2022 •
edited
Loading

Fixes issue #0

Brief summary of changes

Improve performance by:

  • Incorporate fix to cylinder wrap from @adamkewley
  • Improve memory allocation: pre-allocate arrays to avoid copying contents to grow incrementally
  • cache properties into local variables
  • Move loop invariant calculations outside loops
  • Minor cleanup to remove old macros that expand to std functions for clarity
  • leverage the fact that calculation is done in wrapObject's own coordinate system.

Testing I've completed

Looking for feedback on...

CHANGELOG.md (choose one)

  • no need to update because...
  • updated.

This change is Reviewable

Travis CI and others added 30 commits March 16, 2022 12:26
Refactored WrapCylinder::_calc_spiral_wrap_point for const-correctnes…
9a708e0 
…s and shuffled locals around
Reimplemented WrapCylinder::_make_spiral_path for perf
4db52a0
Cleaned up wrap angle calc
f628028
Cleaned up _make_spiral_path comment
637b0b8
Added extremely appropriate TODOs into the wrapping implementation
c51eb7f
@aymanhab
Add test case for testWrappingAlgorithm.cpp
764f70e
@aymanhab
test cyliinder, sphere, ellpsoid path length and moment arms for corr…
a24a33c 
…ectness in nominal setting
@aymanhab
Reduce number of steps and move body COM to allow for path to wrap un…
1ef8383 
…der gravity
@aymanhab
Add non-perpendicular wrapping case
c6c45c1
@aymanhab
Merge branch 'master' into test_wrapping_algorithm
27106c3
@aymanhab
disable rotation tests in-progress
01f3723
@aymanhab
Add test comapre cylinder to ellipsoid wrapping
f5b9d28
@aymanhab
Update continuous_integration.yml
097c925 
Undo cache dependencies on ubuntu since compiler libgc has moved/upgraded
@aymanhab
Update testWrappingAlgorithm.cpp
07d8240 
Fix comment
@aymanhab
Use angles of -30 to 30 degrees for comparing cylinder vs. ellipsoid …
4df447a 
…as larger angles snap incorrectly
@aymanhab
Merge branch 'test_wrapping_algorithm' of https://github.com/opensim-…
1c5ba70 
…org/opensim-core into test_wrapping_algorithm
@aymanhab
Merge branch 'test_wrapping_algorithm' of https://github.com/opensim-…
3a26f15 
…org/opensim-core into test_wrapping_algorithm
@aymanhab
Merge remote-tracking branch 'remotes/origin/master' into test_wrappi…
6c44a91 
…ng_algorithm
@aymanhab
Compare length with analytical answer in every iteration
710f6aa
@aymanhab
Merge branch 'test_wrapping_algorithm' of https://github.com/opensim-…
31e3a26 
…org/opensim-core into test_wrapping_algorithm
@aymanhab
Address feedback on PR, clarify comments and some cleanup
1ffdda4
@aymanhab
Merge branch 'master' into test_wrapping_algorithm
f24787d
@aymanhab
Merge branch 'test_wrapping_algorithm' of https://github.com/opensim-…
f9c84a7 
…org/opensim-core into test_wrapping_algorithm
@aymanhab
Clarify comments and address feedback on test case
c875103
@aymanhab
cache property radius into a member variable _radius
416b9a4
@aymanhab
Merge remote-tracking branch 'remotes/origin/perf_optimize-wrapcylind…
3dfc820 
…er-make-spiral-path' into branch_reduce_propertyaccess_wrap (refactoring of CalcWrapPoint from Adam with additional optimizations for memory management purposes
@aymanhab
Cache property in local variable for speedup
e85d736
@aymanhab
cache properties and call ensureCapacity when size is known to avoid …
9e91120 
…allocation/deallocation/copying overhead
@aymanhab
Fix index 1->i error, cache properties and ensureCapacity on arrays o…
62d29bf 
…nce size is known to avoid allocation/deallocation/copying overhead. Also move loop invariant calculations outside loops
@aymanhab
Vary xyz rotations to exercise quadrants
b7792bf
Base automatically changed from test_wrapping_algorithm to main January 20, 2023 21:47
@adamkewley adamkewley mentioned this pull request Jan 27, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@aymanhab