opensim-org / opensim-gui

SimTK OpenSim graphical user interface and distribution.
Apache License 2.0
61 stars 32 forks source link

GUI Scripts need update: test all and fix or remove from distribution #736

Closed aymanhab closed 5 years ago

aymanhab commented 6 years ago

Jen: Users need examples to show key GUI scripting functionality and at minimum any examples we distribute should work without bugs

Steps to reproduce

Try to run runtutorialOne.py GUI script fails because the models are not in install folder anymore.

Expected result

tutorial runs

Actual result

Exception/error due to not being able to locate model.

Environment and GUI version

Any

aymanhab commented 6 years ago

@chrisdembia The scripts used getInstallDir() which was built into the code, now we need to use the Resources directory instead. I can add a function to get it since we keep it as a Preference. Any thoughts? Do you think we should have the preference exposed?

chrisdembia commented 6 years ago

I think it would be great if the GUI maintained the last location the user specified to install the resources (the preference gets updated), and the user could also view and manually edit this location through the preferences. And yes, it would be great to access this directory within the scripting shell (getResourcesDir() or whatever name is consistent with other preferences/functions). I hope this isn't inconsistent with other suggestions I've given!

I thought the preference was already exposed?

aymanhab commented 6 years ago

Thanks @chrisdembia yes the preference is exposed but that raises a concern that users may inadvertently change it. I'll go ahead and introduce the method so that scripts can be updated.

aymanhab commented 6 years ago

@jimmyDunne can you help update the scripts and report what works and what doesn't so we can either fix or exclude when you have a chance? Thanks

aymanhab commented 6 years ago

@jimmyDunne The method getResourcesDir() has been introduced in commit dated 5/11/18

Please let me know if you run into issues updating the GUI scripts to use it. Thanks

jimmyDunne commented 6 years ago

This is taking a little longer than expected due to changes in the API. For example, the code that changes the path points in the wrist model have significantly changed since 3.3. I am getting through the scripts, but it turned out it isn't so simple. 😭

On the brighter side, getResourcesDir() works great! Thanks, Ayman!

jenhicks commented 6 years ago

@jimmyDunne Let's focus on testing (the sprint tasks) before making big changes to these scripts.

jimmyDunne commented 6 years ago

GUI Scripts

Metabolics Example Scripts

aseth1 commented 6 years ago

Suggestions to be implemented by @jimmyDunne and retested next week. Exclude tutorial 2 and file writing if these remain problematic.

jenhicks commented 6 years ago

Creating a new issues for tutorial 2 ...

scrum2a commented 6 years ago

Leg39 must be verified.

aymanhab commented 6 years ago

@jimmyDunne I'll merge latest master since there're many fixes already there that we don't need to chase unnecessarily. Please let me know otherwise.

jimmyDunne commented 6 years ago

Leg39 is now in the distribution. All the scripts have been updated, and now work as expected.

testShowModelSummary is low priority and can be dealt with in the future.

aymanhab commented 6 years ago

@jimmyDunne do you think this can be safely moved to Needs Verification?

nickbianco commented 6 years ago

I'm working on testing on Windows. However, the Leg39 model is not in the opensim-models branch currently on master. @aymanhab could you update to the correct commit so the Leg39 model is included in the master builds of the GUI?

aymanhab commented 6 years ago

Sure, @nickbianco PR on the way!

aymanhab commented 6 years ago

https://github.com/opensim-org/opensim-gui/pull/988

nickbianco commented 5 years ago

I've tested the scripts with recent fixes on Windows 10, AppVeyor build 1.0.1834, artifact 4.0-2018-09-10-be111ccc. Everything worked fine except for runMultipleIKTrials.py, where one of the MOT files wasn't being printed to the results directory.

I went ahead and created PR #997 to address the issue, which was related to not creating the results directory prior to running the IK tool.

nickbianco commented 5 years ago

Retested with the fix from #997 on AppVeyor build 1.0.1843, Windows 10. runMultipleIKTrials.py is working fine now.

Now that all scripts are tested, I'm moving this issue to Verified on Windows and Mac.