-
Notifications
You must be signed in to change notification settings - Fork 64
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
Gpm null space dimension print #126
Gpm null space dimension print #126
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be nice if this would have been a new feature on a clean branch instead of being added to #124
Could you cherry-pick ipa320@4cccec9 on ìndigo_dev
?
Also, we should have this output for:
- Stack-of-Task:
- Task-2nd-Prio:
4cccec9
to
9f4c64e
Compare
9f4c64e
to
58be1d3
Compare
…ull rank in the order solvers
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
requesting changes
@@ -52,19 +52,29 @@ Eigen::MatrixXd GradientProjectionMethodSolver::solve(const Vector6d_t& in_cart_ | |||
|
|||
Eigen::MatrixXd homogeneous_solution = Eigen::MatrixXd::Zero(particular_solution.rows(), particular_solution.cols()); | |||
KDL::JntArrayVel predict_jnts_vel(joint_states.current_q_.rows()); | |||
Eigen::FullPivLU<Eigen::MatrixXd> lu_decomp(projector); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Performance question:
I think it is quite expensive to perform a FullPivLU to determine rank
Could we not just check whether the determinant of a PartialPivLU is 0.0?
Or even easier:
compare projector
with a Zero-Matrix using isApprox
@@ -106,12 +106,23 @@ Eigen::MatrixXd StackOfTasksSolver::solve(const Vector6d_t& in_cart_velocities, | |||
Eigen::MatrixXd J_temp = J_task * projector_i; | |||
Eigen::VectorXd v_task = it->task_; | |||
Eigen::MatrixXd J_temp_inv = pinv_calc_.calculate(J_temp); | |||
Eigen::FullPivLU<Eigen::MatrixXd> lu_decomp(J_temp_inv); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do you evaluate J_temp_inv
and not projector_i
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because J_temp_inv is going to project the ith task velocities. If it has rank zero is not adding any velocity for that task.
qdots_out.col(0) = q_i; | ||
ROS_WARN("Null space projection matrix is null. It couldn't satisfy all constraints"); | ||
return qdots_out; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we cannot quit here as projector_i = projector_i - J_temp_inv * J_temp;
so even if some projector_i
are Zero, the overall projector_i
not necessarily is Zero as it is a sum not a multiplication.
Just a warning that the projector of the current task is Zero but continue computation
|
||
qdots_out.col(0) = q_i + projector_i * sum_of_gradient; | ||
return qdots_out; | ||
Eigen::FullPivLU<Eigen::MatrixXd> lu_decomp(projector_i); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same performance comment as above
return qdots_out; | ||
} | ||
qdots_out.col(0) = q_i + projector_i * sum_of_gradient; //WHY??? | ||
return qdots_out; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why //WHY???
identation
@@ -64,8 +64,9 @@ Eigen::MatrixXd TaskPrioritySolver::solve(const Vector6d_t& in_cart_velocities, | |||
predict_jnts_vel.q(i) = particular_solution(i, 0) * cycle + joint_states.current_q_(i); | |||
predict_jnts_vel.qdot(i) = particular_solution(i, 0); | |||
} | |||
Eigen::FullPivLU<Eigen::MatrixXd> lu_decomp(projector); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same performance question
|
||
if (this->constraints_.size() > 0) | ||
if ((this->constraints_.size() > 0) && (lu_decomp.rank() != 0)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you put the check into a new if
?
Like this:
if (this->constraints_.size() > 0)
{
if(lu_decomp.rank() != 0)
{
because than it's more clear was has happened, i.e. whether the first or the second condition is false
I played around with this a bit but couldn't enforce a situation where the projection matrix actually becomes Zero, i.e. rank 0 or determinant 0...maybe we have to discuss this a bit more...:wink: |
See the last two commits on my branch where I tested various methods for testing empty Nullspace...
Also, I did quite some indentation fixing |
@ipa-bfb |
@ipa-bfb FYI as discussed in https://github.com/ipa320/cob_control/pull/126#issuecomment-292532450 and because a pointer is added in https://github.com/ipa320/cob_control/issues/152#issuecomment-292532872, I'll close it unmerged...has a merge-conflict anyway |
Solution for issue #125.