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

Fix bugs in PolynomialPathFitter related to the number of input coordinate samples #4039

Merged
merged 7 commits into from
Apr 1, 2025
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ v4.6
other path-based force elements dependent on a user-provided expression. (#4035)
- Fixed a bug where `DeGrooteFregly2016Muscle::getBoundsNormalizedFiberLength()` was returning
tendon force bounds rather than fiber length bounds. (#4040)
- Fixed bugs in `PolynomialPathFitter` when too few coordinate samples were provided. (#4039)


v4.5.1
Expand Down
9 changes: 8 additions & 1 deletion OpenSim/Actuators/PolynomialPathFitter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,12 @@ void PolynomialPathFitter::run() {
"Expected 'num_parallel_threads' to be between 1 and {}, but "
"received {}.", std::thread::hardware_concurrency(),
get_num_parallel_threads())
OPENSIM_THROW_IF_FRMOBJ(
static_cast<int>(values.getNumRows()) < get_num_parallel_threads(),
Exception, "Expected the number of time points in the coordinate "
"values table to be greater than 'num_parallel_threads', but "
"received {} and {}, respectively.",
values.getNumRows(), get_num_parallel_threads())
log_info("Number of parallel threads = {}", get_num_parallel_threads());

// Number of samples per frame.
Expand Down Expand Up @@ -662,7 +668,8 @@ TimeSeriesTable PolynomialPathFitter::sampleCoordinateValues(
int timeIdx = 0;
const auto& times = values.getIndependentColumn();
TimeSeriesTable valuesSampled;
double dt = (times[1] - times[0]) / (get_num_samples_per_frame() + 2);
double dt = (times.size() < 2) ? 0.01 :
(times[1] - times[0]) / (get_num_samples_per_frame() + 2);
for (int i = 0; i < get_num_parallel_threads(); ++i) {
int numTimeIndexes = outputs[i].nrow() / get_num_samples_per_frame();
for (int j = 0; j < numTimeIndexes; ++j) {
Expand Down
19 changes: 17 additions & 2 deletions OpenSim/Actuators/Test/testPolynomialPathFitter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,9 @@ namespace {
return processor;
}

TableProcessor createCoordinatesTable(bool correctLabel, bool addMetaData) {
SimTK::Vector column = SimTK::Test::randVector(100);
TableProcessor createCoordinatesTable(bool correctLabel, bool addMetaData,
int numRows = 100) {
SimTK::Vector column = SimTK::Test::randVector(numRows);
std::vector<double> times;
times.reserve(column.size());
for (int i = 0; i < column.size(); ++i) {
Expand Down Expand Up @@ -130,3 +131,17 @@ TEST_CASE("Invalid configurations") {
"Expected 'maximum_polynomial_order' to be at most 9"));
}
}

TEST_CASE("Number of rows less than the number of threads") {
int numAvailableThreads = std::thread::hardware_concurrency();
int numRows = (numAvailableThreads == 1) ? 1 : numAvailableThreads - 1;
CAPTURE(numAvailableThreads);
CAPTURE(numRows);

PolynomialPathFitter fitter;
fitter.setModel(createHangingMuscleModel());
fitter.setCoordinateValues(createCoordinatesTable(true, true, numRows));
fitter.setNumParallelThreads(numAvailableThreads);
REQUIRE_THROWS_WITH(fitter.run(), ContainsSubstring(
"Expected the number of time points in the coordinate values table"));
}
Loading