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

introduce joint jerk bounds #76

Merged
merged 3 commits into from
Jul 22, 2021
Merged

introduce joint jerk bounds #76

merged 3 commits into from
Jul 22, 2021

Conversation

ndehio
Copy link
Contributor

@ndehio ndehio commented Jul 19, 2021

Analogously to the existing joint acceleration bounds, this PR introduces joint jerk bounds.
For some robots these limits are defined by the manufacturer, see for example the panda manipulator.

@gergondet
Copy link
Member

I don't think this implementation is correct. This is bounding the acceleration to be lower than (jerk bound)*dt but that's not the same as bounding the jerk. This imposes more restrictive acceleration bounds without preventing jerk limits violations.

For example with the Panda jerk bounds it limits the acceleration to half of the actual acceleration bounds but if the acceleration goes from the minimum bound (-jerk_max * dt) to the minimum bound (jerk_max * dt) in one iteration this would generate a 2*jerk_max jerk (it's unlikely to happen but that would generate the maximum jerk allowed by the constraint).

@ndehio
Copy link
Contributor Author

ndehio commented Jul 21, 2021

Oh right, similar to the torque-derivative constraint that considers the current torques, we here need to involve the current joint accelerations for the jerk constraint.
I added a member for that purpose, but it is not clear to me how to update it. Can we do it like this?

rbd::paramToVector(mbc.alphaD, curAlphaD_); //line 118 of QPConstr.cpp

and

curAlphaD_[d.alphaDBegin] = mbc.alphaD[d.jointIndex][0]; //line 252 of QPConstr.cpp

Copy link
Member

@gergondet gergondet left a comment

Choose a reason for hiding this comment

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

My main complaint would be to rename curAlphaD_ to prevAlphaD_ in both case and make sure to update it

src/QPConstr.cpp Outdated
@@ -75,6 +76,7 @@ JointLimitsConstr::JointLimitsConstr(const std::vector<rbd::MultiBody> & mbs,
alphaDDUpper_.resize(mb.nrDof());
alphaDDLower_.setConstant(-std::numeric_limits<double>::infinity());
alphaDDUpper_.setConstant(std::numeric_limits<double>::infinity());
curAlphaD_.resize(mb.nrDof());
Copy link
Member

Choose a reason for hiding this comment

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

curAlphaD_.setZero();

src/QPConstr.cpp Show resolved Hide resolved
src/QPConstr.cpp Show resolved Hide resolved
src/QPConstr.cpp Outdated
@@ -105,6 +124,8 @@ void JointLimitsConstr::update(const std::vector<rbd::MultiBody> & /* mbs */,

lower_ = lower_.cwiseMax(alphaDLower_);
upper_ = upper_.cwiseMin(alphaDUpper_);
lower_ = lower_.cwiseMax((alphaDDLower_ * step_) + curAlphaD_);
upper_ = upper_.cwiseMin((alphaDDUpper_ * step_) + curAlphaD_);
Copy link
Member

Choose a reason for hiding this comment

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

curAlphaD_ needs to be updated (and I would rename it prevAlphaD_ I think this is clearer)

src/QPConstr.cpp Show resolved Hide resolved
src/QPConstr.cpp Outdated
@@ -238,6 +287,8 @@ void DamperJointLimitsConstr::update(const std::vector<rbd::MultiBody> & /* mbs
}
lower_ = lower_.cwiseMax(alphaDLower_);
upper_ = upper_.cwiseMin(alphaDUpper_);
lower_ = lower_.cwiseMax((alphaDDLower_ * step_) + curAlphaD_);
upper_ = upper_.cwiseMin((alphaDDUpper_ * step_) + curAlphaD_);
Copy link
Member

Choose a reason for hiding this comment

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

Same remark regarding curAlphaD_ update and renaming to prevAlphaD_

@gergondet
Copy link
Member

I added a member for that purpose, but it is not clear to me how to update it. Can we do it like this?

rbd::paramToVector(mbc.alphaD, curAlphaD_); //line 118 of QPConstr.cpp

Yes. This works to update the full curAlphaD_ (or rather prevAlphaD_)

and

curAlphaD_[d.alphaDBegin] = mbc.alphaD[d.jointIndex][0]; //line 252 of QPConstr.cpp

No. This would be wrong. You only need the first line to update prevAlphaD_ after the update in both JointLimitsConstr and DamperJointLimitsConstr

@ndehio
Copy link
Contributor Author

ndehio commented Jul 21, 2021

Thanks @gergondet for your comments, I applied them all.

@gergondet gergondet merged commit d386707 into jrl-umi3218:master Jul 22, 2021
gergondet added a commit that referenced this pull request Jul 30, 2021
- Add joint jerk bounds support (#76)
- Add tasks::qp::PostureTask::dimWeight (#77)
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