opensim-core icon indicating copy to clipboard operation
opensim-core copied to clipboard

Published 20 hours ago • verified publisher icon opensim-org

Beefup markersreference testcase

Open aymanhab opened this issue 2 years ago • 5 comments

Fixes issue #3226

Brief summary of changes

Exercise workflow in issue 3226

Testing I've completed

Looking for feedback on...

CHANGELOG.md (choose one)

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

This change is Reviewable

aymanhab avatar Jun 20 '22 20:06 aymanhab

@CVHammond this PR exercises the code you have in #3226, I will not merge it since it tests nothing now but it doesn't crash. If you agree, this points to memory management issues/early garbage collection in Matlab rather than an actual API bug, I will propose ideas/suggestions if that proves to be the case.

aymanhab avatar Jun 20 '22 20:06 aymanhab

@aymanhab that appears to be the correct C++ implementation of the MATLAB test case that I posted. Looking forward to advice on adjustments to deal with MATLAB's memory management

cvhammond avatar Jun 20 '22 20:06 cvhammond

Some ideas for troubleshooting:

  1. print the version (opensimCommonJNI.GetVersionAndDate()) to make sure the wiring is to the latest build of 4.4.
  2. Create objects and keep references to them in variables rather than on the stack e.g. SimTKArrayCoordinateReference(), otherwise they could be immediately garbage collected.
  3. Where does the crash happen, if in a loop is it at a specific index?
  4. If there's a stacktrace that would be extremely helpful to pinpoint the issue source.
  5. I understand this is a simplified version, which is best but I don't know if it actually crashes for you or if the crash happens only in more complicated setting where the three sets (Model-makers vs. weights vs. markers-in-trc-file) are not identical.
  6. I don't have a working Matlab environment but can get that setup over the next day or two. In that case I'd like to know your Matlab version.

Let me know what you find out or if you get more clues on what's going wrong.

aymanhab avatar Jun 20 '22 23:06 aymanhab

I believe I have found the issue and fixed it. It was a combination of the MarkersReference object requiring a specific signature and the MarkerReference's MarkerTable returning double sometimes and java.lang.double other times for MarkerTable.getNearestRowIndexForTime(). The suggestion to make sure I was using the HEAD (opensimCommonJNI.GetVersionAndDate()) was important as well. I'm learning more about interacting with the C++ API than I originally thought I would.

All of my tests now pass as they did on 4.0.

cvhammond avatar Jun 21 '22 20:06 cvhammond

Outstanding! thanks for the update. The issue with double vs. java.lang.double was reported by another user earlier and we couldn't reproduce. If you can provide a minimal example that would be awesome. Thanks for your help 👍

aymanhab avatar Jun 21 '22 20:06 aymanhab