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.

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

Triangle Inequality Fix #183

Merged
merged 1 commit into from
Dec 9, 2024

Conversation

alexbeattie42
Copy link
Contributor

Resolves this issue

I created this python script which identifies in a .osim model which inertial constraints will fail the triangle inequality and calculates the minimum change in one element required to fix them. I have tested with the models before and after and in my testing the models do not work prior to the repair and do work after.

@alexbeattie42 alexbeattie42 force-pushed the triangle-inequality-fix branch from c2a5b1c to e0cee6c Compare December 6, 2024 05:55
@aymanhab
Copy link
Member

aymanhab commented Dec 6, 2024

LGTM, thanks @alexbeattie42 👍

@aymanhab
Copy link
Member

aymanhab commented Dec 6, 2024

@nickbianco Do you think these minimal changes in inertia would break any tests/regressions or prevent reproduction of published work?

@nickbianco
Copy link
Member

@aymanhab I don't think we have any regression tests based directly on these models, and I doubt that these small changes would make much of difference in simulation. Users should be packaging models they used to create their simulations for reproducibility.

I'm still confused on why this inertia error is appearing all of a sudden. I can't think of any changes in OpenSim or Simbody that would lead to this. That said, the changes seem reasonable enough.

@aymanhab
Copy link
Member

aymanhab commented Dec 6, 2024

@nickbianco Using the latest version of the GUI build/artifact from here:
https://github.com/opensim-org/opensim-gui/actions/runs/12131796667

I can open the Rajagopal2016 model without an issue on windows, and we don't support/publish a linux GUI. My guess is that these numerical differences are platform dependent, maybe due to lapack/blas differences so i see little harm in making the change but let me know otherwise.

@aymanhab aymanhab merged commit 20a888b into opensim-org:master Dec 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SimTK Execption - Inertial Matrix Triangle Inequality Bug
3 participants