From 6b111ef953c324a12a017938f728a845b8a860f5 Mon Sep 17 00:00:00 2001 From: aymanhab Date: Mon, 18 Dec 2023 19:47:59 -0800 Subject: [PATCH 1/6] Undo binding of model getCoordinatesInMultibodyTreeOrder and introduce getCoordinateNamesInMultibodyTreeOrder for scripting users --- Bindings/Python/tests/test_simbody.py | 5 +++++ Bindings/simulation.i | 3 --- OpenSim/Simulation/Model/Model.h | 8 ++++++++ 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/Bindings/Python/tests/test_simbody.py b/Bindings/Python/tests/test_simbody.py index f6636d9ebd..b3472f7cd4 100644 --- a/Bindings/Python/tests/test_simbody.py +++ b/Bindings/Python/tests/test_simbody.py @@ -205,6 +205,9 @@ def test_SimbodyMatterSubsystem(self): assert (smss.calcSystemMassCenterLocationInGround(s)[2] == model.calcMassCenterPosition(s)[2]) + coordNames = model.getCoordinateNamesInMultibodyTreeOrder(); + print('firstCoord', coordNames.get(0)); + J = osim.Matrix() smss.calcSystemJacobian(s, J) # 6 * number of mobilized bodies @@ -248,3 +251,5 @@ def test_SimbodyMatterSubsystem(self): assert residual.size() == s.getNU() for i in range(residual.size()): assert abs(residual[i]) < 1e-10 + + diff --git a/Bindings/simulation.i b/Bindings/simulation.i index 29385e4cbf..906b5d8c64 100644 --- a/Bindings/simulation.i +++ b/Bindings/simulation.i @@ -177,9 +177,6 @@ OpenSim::ModelComponentSet; %include %include %copyctor OpenSim::Model; -%include -%template(ReferencePtrCoordinate) SimTK::ReferencePtr; -%template(StdVectorReferencePtrCoordinate) std::vector>; %include %include diff --git a/OpenSim/Simulation/Model/Model.h b/OpenSim/Simulation/Model/Model.h index e2a67e32f8..3fbd0e3dd1 100644 --- a/OpenSim/Simulation/Model/Model.h +++ b/OpenSim/Simulation/Model/Model.h @@ -943,6 +943,14 @@ OpenSim_OBJECT_NONTEMPLATE_DEFS(Model, ModelComponent); std::vector> getCoordinatesInMultibodyTreeOrder() const; + /** A variant of getCoordinatesInMultibodyTreeOrder that returns names for Scripting users */ + SimTK::Array_ getCoordinateNamesInMultibodyTreeOrder() { + SimTK::Array_ namesArray; + auto coords = getCoordinatesInMultibodyTreeOrder(); + for (auto coord : coords) + namesArray.push_back(coord->getName()); + return namesArray; + } /** Get a warning message if any Coordinates have a MotionType that is NOT consistent with its previous user-specified value that existed in Model files prior to OpenSim 4.0 */ From 2995259dded997aa4e4788526589ad1aea39b590 Mon Sep 17 00:00:00 2001 From: aymanhab Date: Tue, 19 Dec 2023 10:41:56 -0800 Subject: [PATCH 2/6] Fix dereferencing the ReferencePtr and exercise in test case --- OpenSim/Simulation/Model/Model.h | 4 ++-- OpenSim/Simulation/Test/testAssemblySolver.cpp | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/OpenSim/Simulation/Model/Model.h b/OpenSim/Simulation/Model/Model.h index 3fbd0e3dd1..e205b8f863 100644 --- a/OpenSim/Simulation/Model/Model.h +++ b/OpenSim/Simulation/Model/Model.h @@ -947,8 +947,8 @@ OpenSim_OBJECT_NONTEMPLATE_DEFS(Model, ModelComponent); SimTK::Array_ getCoordinateNamesInMultibodyTreeOrder() { SimTK::Array_ namesArray; auto coords = getCoordinatesInMultibodyTreeOrder(); - for (auto coord : coords) - namesArray.push_back(coord->getName()); + for (int i=0; i < coords.size(); ++i) + namesArray.push_back(coords[i]->getName()); return namesArray; } /** Get a warning message if any Coordinates have a MotionType that is NOT diff --git a/OpenSim/Simulation/Test/testAssemblySolver.cpp b/OpenSim/Simulation/Test/testAssemblySolver.cpp index 6624abf2fa..9571875d4a 100644 --- a/OpenSim/Simulation/Test/testAssemblySolver.cpp +++ b/OpenSim/Simulation/Test/testAssemblySolver.cpp @@ -131,6 +131,8 @@ void testAssembleModelWithConstraints(string modelFile) for(int i=0; i< coords.getSize(); i++) { cout << "Coordinate " << coords[i].getName() << " get value = " << coords[i].getValue(state) << endl; } + auto coordsInOrder = model.getCoordinateNamesInMultibodyTreeOrder(); + cout << coordsInOrder << std::endl; // Initial coordinates after initial assembly Vector q0 = state.getQ(); From 02f5ec986d6828d469b4fb1f1e86a439509eff73 Mon Sep 17 00:00:00 2001 From: aymanhab Date: Tue, 19 Dec 2023 12:34:50 -0800 Subject: [PATCH 3/6] Fix compilation warnings/errors on linux and call site to index the SimTKArray --- Bindings/Python/tests/test_simbody.py | 2 +- OpenSim/Simulation/Model/Model.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Bindings/Python/tests/test_simbody.py b/Bindings/Python/tests/test_simbody.py index b3472f7cd4..d03fddd6ce 100644 --- a/Bindings/Python/tests/test_simbody.py +++ b/Bindings/Python/tests/test_simbody.py @@ -206,7 +206,7 @@ def test_SimbodyMatterSubsystem(self): model.calcMassCenterPosition(s)[2]) coordNames = model.getCoordinateNamesInMultibodyTreeOrder(); - print('firstCoord', coordNames.get(0)); + print('firstCoord', coordNames.getElt(0)); J = osim.Matrix() smss.calcSystemJacobian(s, J) diff --git a/OpenSim/Simulation/Model/Model.h b/OpenSim/Simulation/Model/Model.h index e205b8f863..a599a7920c 100644 --- a/OpenSim/Simulation/Model/Model.h +++ b/OpenSim/Simulation/Model/Model.h @@ -947,7 +947,7 @@ OpenSim_OBJECT_NONTEMPLATE_DEFS(Model, ModelComponent); SimTK::Array_ getCoordinateNamesInMultibodyTreeOrder() { SimTK::Array_ namesArray; auto coords = getCoordinatesInMultibodyTreeOrder(); - for (int i=0; i < coords.size(); ++i) + for (auto i=0; i < coords.size(); ++i) namesArray.push_back(coords[i]->getName()); return namesArray; } From 068bbe8a31351dd7d62d369857c052dde8bd5817 Mon Sep 17 00:00:00 2001 From: aymanhab Date: Tue, 19 Dec 2023 12:50:35 -0800 Subject: [PATCH 4/6] Fix int/size_t mixup that causes compilation errors on linux --- OpenSim/Simulation/Model/Model.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/OpenSim/Simulation/Model/Model.h b/OpenSim/Simulation/Model/Model.h index a599a7920c..f70c4d666c 100644 --- a/OpenSim/Simulation/Model/Model.h +++ b/OpenSim/Simulation/Model/Model.h @@ -947,7 +947,7 @@ OpenSim_OBJECT_NONTEMPLATE_DEFS(Model, ModelComponent); SimTK::Array_ getCoordinateNamesInMultibodyTreeOrder() { SimTK::Array_ namesArray; auto coords = getCoordinatesInMultibodyTreeOrder(); - for (auto i=0; i < coords.size(); ++i) + for (int i=0; i < (int)coords.size(); ++i) namesArray.push_back(coords[i]->getName()); return namesArray; } From 30f0de17f1733764b3973e473f9fbdcc0969f1f7 Mon Sep 17 00:00:00 2001 From: Ayman Habib Date: Tue, 19 Dec 2023 14:53:51 -0800 Subject: [PATCH 5/6] Update CHANGELOG.md Use updated function name --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 95fbd56d1b..12e6d173d9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,7 +25,7 @@ and `Blankevoort1991Ligament`, usages of `get_GeometryPath`, `upd_GeometryPath`, - Deleting elements from an `OpenSim::Coordinate` range now throws an exception during `finalizeFromProperties` (previously: it would let you do it, and throw later when `Coordinate::getMinRange()` or `Coordinate::getMaxRange()` were called, #3532) - Added `FunctionBasedPath`, a class for representing paths in `Force`s based on `Function` objects (#3389) -- Fixed bindings to expose the method Model::getCoordinatesInMultibodyTreeOrder to scripting users (#3569) +- Fixed bindings to expose the method Model::getCoordinateNamesInMultibodyTreeOrder to scripting users (#3569) - Fixed a bug where constructing a `ModelProcessor` from a `Model` object led to an invalid `Model` - Added `LatinHypercubeDesign`, a class for generating Latin hypercube designs using random and algorithm methods (#3570) - Refactor c3dExport.m file as a Matlab function (#3501), also expose method to allow some operations on tableColumns From 3e345e4a2ee67f1c34f809be5e0b59c97baf9fbe Mon Sep 17 00:00:00 2001 From: aymanhab Date: Tue, 19 Dec 2023 15:21:11 -0800 Subject: [PATCH 6/6] Address feedback on PR in test, changelog language --- CHANGELOG.md | 2 +- OpenSim/Simulation/Test/testAssemblySolver.cpp | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 12e6d173d9..f66afa9849 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,7 +25,7 @@ and `Blankevoort1991Ligament`, usages of `get_GeometryPath`, `upd_GeometryPath`, - Deleting elements from an `OpenSim::Coordinate` range now throws an exception during `finalizeFromProperties` (previously: it would let you do it, and throw later when `Coordinate::getMinRange()` or `Coordinate::getMaxRange()` were called, #3532) - Added `FunctionBasedPath`, a class for representing paths in `Force`s based on `Function` objects (#3389) -- Fixed bindings to expose the method Model::getCoordinateNamesInMultibodyTreeOrder to scripting users (#3569) +- Introduced the method `Model::getCoordinateNamesInMultibodyTreeOrder` for convenient access to internal coordinate ordering for scripting users (#3569) - Fixed a bug where constructing a `ModelProcessor` from a `Model` object led to an invalid `Model` - Added `LatinHypercubeDesign`, a class for generating Latin hypercube designs using random and algorithm methods (#3570) - Refactor c3dExport.m file as a Matlab function (#3501), also expose method to allow some operations on tableColumns diff --git a/OpenSim/Simulation/Test/testAssemblySolver.cpp b/OpenSim/Simulation/Test/testAssemblySolver.cpp index 9571875d4a..b19b4bb0c7 100644 --- a/OpenSim/Simulation/Test/testAssemblySolver.cpp +++ b/OpenSim/Simulation/Test/testAssemblySolver.cpp @@ -134,6 +134,7 @@ void testAssembleModelWithConstraints(string modelFile) auto coordsInOrder = model.getCoordinateNamesInMultibodyTreeOrder(); cout << coordsInOrder << std::endl; + assert(coords.getSize()==coordsInOrder.size()); // Initial coordinates after initial assembly Vector q0 = state.getQ();