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

Change linear friction cone to ice cream cone #238

Merged

Conversation

edantec
Copy link
Collaborator

@edantec edantec commented Oct 24, 2024

No description provided.

CHANGELOG.md Outdated Show resolved Hide resolved
Copy link
Member

@ManifoldFR ManifoldFR left a comment

Choose a reason for hiding this comment

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

Just some questions

include/aligator/solvers/proxddp/solver-proxddp.hxx Outdated Show resolved Hide resolved
@@ -51,7 +51,7 @@ void exposeContactForce() {
.def(bp::init<int, PinModel, const context::MatrixXs &,
const RigidConstraintModelVector &,
const pinocchio::ProximalSettingsTpl<Scalar> &,
const context::Vector6s &, const std::string &>(bp::args(
const context::VectorXs &, const std::string &>(bp::args(
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand this change

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In this context, the reference force can be a 3D or 6D vector, depending on the type of contact. The bindings were not covering the 3D case.

Copy link
Member

Choose a reason for hiding this comment

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

Okay, so in this case, since at most it can be of dimension six, you could use the Eigen::Matrix template with maximum number of rows at compile time set to 6, in both the binding code and the actual C++ class:

using Vector3or6 = Eigen::Matrix<Scalar, -1, 1, 6>;

The fourth template parameter is known in Eigen as MaxRowsAtCompileTime.
This allows us to avoid an unnecessary heap allocation in this case.

Copy link
Member

@ManifoldFR ManifoldFR left a comment

Choose a reason for hiding this comment

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

superb work

@ManifoldFR ManifoldFR merged commit 44cabf0 into Simple-Robotics:devel Nov 4, 2024
15 checks passed
@edantec edantec deleted the topic/edantec_icecream_cone branch November 4, 2024 17:14
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