Backport scale opensim33 #538
Conversation
|
Fixes #186 |
|
@chrisdembia travis has been failing since yesterday trying to get swig from http://prdownloads.sourceforge.net/swig/$SWIG.tar.gz do we have a workaround, different location to obtain swig or just disable wrapping for now since these PRs have no API changes? Thanks. |
|
Is the failure sporadic? The travis-ci/push test passed. I restarted the build. Yes it would be a good idea to download swig from somewhere else; I guess we can't rely on sourceforge. At one point, we briefly downloaded swig from github. That was a good idea because then if github is down, we have more issues than just not being able to download swig. I don't know why I reverted the changes. |
|
I believe the problem was an outage yesterday on sourceforge. I think it is fixed now. |
|
Sherm, Apparently it isn't fixed yet as travis just failed to build master for the -Ayman On Fri, Jul 17, 2015 at 10:51 AM, Michael Sherman notifications@github.com
|
Yes, I see I was wrong. Sourceforge is back up but prdownloads.sourceforge.net is still dead. |
|
@aseth1 this ports the scaling of ligaments and test case from 3.3 branch. Please review and let me know if ready to merge. Thanks |
There was a problem hiding this comment.
The tool should not be called "subject"! Becomes confusing to read below.
|
@aseth1 Changes made per your suggestion (name of local variable for tool and move of check/assert block). The test case is validated against manual inspection for a simple block with attached ligaments. There's no existing code that would have scaled ligament resting length properly, the scaling of geometryPath for this model is trivial and has been visually verified by @jimmyDunne and myself. Please let me know if you have any further feedback or if we should merge. Thanks. |
There was a problem hiding this comment.
Does this actually remove the file or just cause it to be empty?
There was a problem hiding this comment.
You're correct, it empties the file rather than removes it. I'll update the comment(s) accordingly.
I forgot about the resting length too. If the std has been verified to have the correct resting length (and it isn't equivalent to the unscaled) then that's OK. I was referring to testing if the scale factors for the ligament are being computed consistent with a muscle. I thought that would be a simple check to add by comparing the ligament to a muscle with the identical path. |
|
@aseth1 Indeed we can add muscles in parallel with ligaments to test the path scaling as a test for self-consistency but considering that both Ligaments and Muscles delegate their path scaling to GeometryPath.scale() this may not strengthen the test case much as it stands. Please let me know your preference. Thanks. |
I agree we should have an issue to discuss scaling going forward. One idea is that Frame contains scale factors (like it is now, which are determined by a ModelScaler) and then it is up to Components that depend on (connect to) Frames to use these values to perform their own scaling. It needs work. |
Fix typo.
|
Can we merge this, @aseth1 ? Thanks |
|
I am reluctantly merging this as a necessary band-aid to address Ligaments alone. There remains a glaring issue that other Components (esp. forces connected to bodies) will not be scaled, which is beyond the intent of this PR. |
|
Thanks @aseth1 Please add your comments/issues to issue #557 so the discussion doesn't fall through the cracks. Maybe @jimmyDunne can pick it up when working on fixer. |
|
🎱 |
…rdinates are accessible during Model building. It does not resolve the problem that coordinates of the joint are not accessible upon construction of the Joint (see #538). Temporary fix is to make a call to finalizeFromProperties.
Backport changes to scaling from OpenSim 3.3 code base. This amounts to scaling ligaments by invoking their (PreScale, Scale, PostScale methods) and beefing up the test case to check that ligaments are scaled as expected.