Computing marker RMS uses the number of non-NaN values #2059
Conversation
…turned by InverseKinematicsSolver is consistent with order of marker errors.
|
@aseth1 I have a model and a TRC file for the case that the model has fewer markers than the TRC file. I do not have data that has NaNs. |
There was a problem hiding this comment.
I like the improved RMS reporting in this PR. I made one small comment. I think it's fine that there is not an explicit test for the updated RMS calculation, but it would be good to just manually inspect that the RMS is larger with this change. Doing so is difficult, because introducing NaNs into the TRC file also changes the IK solution. It seems that verifying this fix requires using the setup you have in the IKSolver test, where a marker is not in the plane of the multibody system.
|
|
||
| ikSolver.getMarkerReferenceValues(markerRefValues); | ||
| for (const Vec3& val : markerRefValues) { | ||
| val.isNaN() ? ignore_cnt++ : 0; |
There was a problem hiding this comment.
I think it makes more sense to write this as if (val.isNaN()) ++ignore_cnt; because the value of the ? : expression is not used.
There was a problem hiding this comment.
Does this count skip over references for markers that do not have a corresponding model marker?
There was a problem hiding this comment.
Does this count skip over references for markers that do not have a corresponding model marker?
Markers without a corresponding model marker are not part of the reference values returned because they are not added to the solver to begin with.
There was a problem hiding this comment.
Okay thanks for clarifying.
|
@jimmyDunne do you have a trial with NaNs handy? We need to verify that the RMS error reported increases when markers drop out (instead of getting more accurate, which was wrong). |
|
Attached is an edited trc file and corresponding model. The trc file is the one distributed with gait2392. |
|
@aseth1, it seems like some conflicts would need to be fixed but, is this still something we should try to get in? |
|
@jimmyDunne I thought this got merged ages ago! I can resolve the conflicts if you are willing to verify on a real example. The test case included makes sure that NaNs are ignored and the number of markers is reduced at that instant, so that the mean error is computed over the correct number. I am very confident it is correct, but would be great to verify. |
Fixes issue #2058
Brief summary of changes
Added methods to access the marker reference values from the
InverseKinematicsSolver.Using this to check if any reference value is NaN when accounting for it when reporting marker RMS errors.
Testing I've completed
Added conditions to testInverseKinematicsSolver to check that NaN reference values are being returned and correspond to the correct marker.
There isn't an explicit test for running the IKTool with with experimental data with NaNs.
Looking for feedback on...
@jimmyDunne , @chrisdembia , @apoorvar perhaps you have a model and data handy for me to use as a test case?
CHANGELOG.md (choose one)
This change is