Finishing the refactor...#417
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR completes a refactor to make Kinematics, Mechanics, Statics, and the new Dynamics factor-builders follow the same context pattern (Slice / Interval / Phase), while reducing duplication in DynamicsGraph and aligning parameter hierarchies.
Changes:
- Introduces a new
Mechanicslayer (andMechanicsParameters) to centralize shared wrench/torque factor groups for statics/dynamics. - Introduces a new
Dynamicsfactor builder (withDynamicsParameters) and delegates slice/interval/phase dynamics assembly out ofDynamicsGraph. - Adds
ContextUtilshelpers to assemble per-slice factor graphs/values across interval-like contexts; updates wrappers and adds an architecture plan doc.
Reviewed changes
Copilot reviewed 28 out of 28 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| gtdynamics/utils/ContextUtils.h | Adds utilities to iterate contexts and accumulate per-slice graphs/values. |
| gtdynamics/statics/StaticsSlice.cpp | Converts slice statics API to explicit template specializations; removes direct factor includes. |
| gtdynamics/statics/StaticsInterval.cpp | Implements interval statics via collectFactors/collectValues. |
| gtdynamics/statics/StaticsPhase.cpp | Implements phase statics by delegating to interval. |
| gtdynamics/statics/Statics.h | Makes Statics compose shared mechanics factor groups via Mechanics; updates parameter inheritance. |
| gtdynamics/mechanics/Mechanics.h | Introduces shared mechanics factor builder interface (templated-by-context). |
| gtdynamics/mechanics/MechanicsParameters.h | Introduces shared parameter bundle for mechanics-level factor groups. |
| gtdynamics/mechanics/MechanicsSlice.cpp | Implements mechanics factor groups for a single slice. |
| gtdynamics/mechanics/MechanicsInterval.cpp | Implements mechanics factor groups across an interval by collecting slices. |
| gtdynamics/mechanics/MechanicsPhase.cpp | Implements mechanics factor groups across a phase by delegating to interval. |
| gtdynamics/mechanics/CMakeLists.txt | Placeholder note (sources pulled via top-level globbing). |
| gtdynamics/kinematics/Kinematics.h | Expands documentation and context-template API surface for q/v factor helpers. |
| gtdynamics/kinematics/KinematicsSlice.cpp | Moves slice q/v factor implementations to explicit template specializations. |
| gtdynamics/kinematics/KinematicsInterval.cpp | Refactors interval implementations to use collectFactors/collectValues; adds q/v interval specializations. |
| gtdynamics/kinematics/KinematicsPhase.cpp | Adds q/v phase specializations delegating to interval. |
| gtdynamics/dynamics/Dynamics.h | Introduces Dynamics factor builder class with context-templated API. |
| gtdynamics/dynamics/DynamicsParameters.h | Introduces DynamicsParameters (inherits MechanicsParameters) and migrates dynamics noise models. |
| gtdynamics/dynamics/DynamicsSlice.cpp | Implements slice acceleration factors and dynamic-only wrench/contact factors. |
| gtdynamics/dynamics/DynamicsInterval.cpp | Implements interval dynamics via per-slice collection. |
| gtdynamics/dynamics/DynamicsPhase.cpp | Implements phase dynamics by delegating to interval. |
| gtdynamics/dynamics/MechanicsParameters.h | Adds backwards-compatible include for moved mechanics parameters. |
| gtdynamics/dynamics/OptimizerSetting.h | Changes inheritance to include DynamicsParameters; removes in-class factor model members. |
| gtdynamics/dynamics/OptimizerSetting.cpp | Updates default ctor to use DynamicsParameters() instead of initializing all models locally. |
| gtdynamics/dynamics/DynamicsGraph.h | Stores Mechanics and Dynamics members; adds configuredSettings helper. |
| gtdynamics/dynamics/DynamicsGraph.cpp | Delegates aFactors/dynamics-only factor assembly to Dynamics and shared groups to Mechanics. |
| gtdynamics/CMakeLists.txt | Adds mechanics directory to library source subdirs. |
| gtdynamics.i | Updates wrapper exposure for moved/added parameter types and new Dynamics builder. |
| doc/kinodynamics-architecture-refactor-plan.md | Adds a detailed phased refactor plan and architecture guidance. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Now Kinematics/Mechanics/Statics/Dynamics work in the same way.
Next up after this PR: move all the factors (and tests), and try to remove all direct uses of factors outside the kinematics/Mmechanics/statics/dynamics folders, to keep the code base modular.