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.

@ManifoldFR ManifoldFR force-pushed the topic/edantec_icecream_cone branch from a1a4f0f to b9eb358 Compare October 24, 2024 21:36
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

@@ -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.

@edantec edantec force-pushed the topic/edantec_icecream_cone branch from 8aca851 to bf5b70c Compare October 31, 2024 10:23
@edantec edantec force-pushed the topic/edantec_icecream_cone branch from bf5b70c to e080d61 Compare November 4, 2024 10:10
@ManifoldFR ManifoldFR enabled auto-merge November 4, 2024 16:53
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