Skip to content

Refactor PolynomialPathFitter#4302

Open
nickbianco wants to merge 1 commit intoopensim-org:mainfrom
nickbianco:path_fitter_refactor
Open

Refactor PolynomialPathFitter#4302
nickbianco wants to merge 1 commit intoopensim-org:mainfrom
nickbianco:path_fitter_refactor

Conversation

@nickbianco
Copy link
Copy Markdown
Member

@nickbianco nickbianco commented Apr 9, 2026

Partially addresses issue #4291

Brief summary of changes

This change refactors PolynomialPathFitter to support future improvements as described in issue #4291.

  • Moved the private helper function choose() to CommonUtilities.
  • Remove all remaining private helper functions to an anonymous namespace in PolynomialPathFitter.cpp.
  • Added the helper types CoordinateMap and Settings for passing configuration information to various parts of the path fitting pipeline.

Other minor changes include:

  • Removed various instances of trailing whitespace in CommonUtilities.

Testing I've completed

Ran testPolynomialPathFitter.cpp.

Looking for feedback on...

CHANGELOG.md (choose one)

  • no need to update because...internal refactor.

This change is Reviewable

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Refactors PolynomialPathFitter internals to make the path-fitting pipeline more modular and easier to evolve (per issue #4291), including moving shared helpers into CommonUtilities.

Changes:

  • Adds a new public OpenSim::choose(n, k) utility in CommonUtilities.
  • Moves PolynomialPathFitter pipeline helpers into an anonymous namespace and introduces internal Settings/CoordinateMap for configuration passing.
  • Centralizes property validation into a new validateProperties() helper and updates data/model handling within run().

Reviewed changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 5 comments.

File Description
OpenSim/Common/CommonUtilities.h Declares new choose() utility and cleans up trailing whitespace.
OpenSim/Common/CommonUtilities.cpp Defines choose() implementation.
OpenSim/Actuators/PolynomialPathFitter.h Updates header guard, refactors private helpers, adds runtime members and validation hook.
OpenSim/Actuators/PolynomialPathFitter.cpp Refactors pipeline into internal helpers with Settings passed through; adds validateProperties() and updates run() flow.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread OpenSim/Common/CommonUtilities.cpp
Comment thread OpenSim/Actuators/PolynomialPathFitter.cpp Outdated
Comment thread OpenSim/Actuators/PolynomialPathFitter.cpp Outdated
Comment thread OpenSim/Actuators/PolynomialPathFitter.h Outdated
Comment thread OpenSim/Actuators/PolynomialPathFitter.cpp Outdated
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread OpenSim/Common/CommonUtilities.cpp
Comment thread OpenSim/Actuators/PolynomialPathFitter.cpp
@nickbianco nickbianco force-pushed the path_fitter_refactor branch from c6b743a to c0a2dd8 Compare April 10, 2026 19:51
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.

2 participants