stephane-caron / pink

Python inverse kinematics based on Pinocchio
Apache License 2.0
246 stars 15 forks source link

Introduction of Barriers, Position barrier #87

Closed domrachev03 closed 4 months ago

domrachev03 commented 4 months ago

This PR is first from the series of PRs, aimed at introduction of Control Barrier Functions, mentioned in #86 . introduces safety sets in the form of Control Barrier Function (CBF) in the presented framework.

For the theory, see our fork, particularly this Jupyter Notebook.

As for implementation, each Barrier defines constraint, and might also define penalization term, if corresponding weight is non-zero. To do this, each barrier has to be able to compute it's value, jacobian, and (optionally) recovery velocity.

Features:

  1. General notion of barrier.
  2. Position barrier for given frame.
  3. Unit tests that cover 100% of presented functionality.

Examples:

  1. Manipulator UR5 with limiting y-plane.
  2. Quadruped Robot Go2 with limiting x and y planes.

Note that this code already fixes problems, mentioned in the #86. Any further suggestions would be welcome

coveralls commented 4 months ago

Pull Request Test Coverage Report for Build 9282083375

Details


Changes Missing Coverage Covered Lines Changed/Added Lines %
tests/test_barrier.py 45 46 97.83%
<!-- Total: 201 202 99.5% -->
Totals Coverage Status
Change from base Build 9045327765: 0.06%
Covered Lines: 1443
Relevant Lines: 1459

💛 - Coveralls
domrachev03 commented 4 months ago

Good evening. We've fixed most of the suggested points. Moreover, we've started a discussion here.

Waiting for your answer, while @simeon-ned is finalising the documentation on his website.

domrachev03 commented 4 months ago

Thank you for your patience and attention, it is great to see so much involvement in the process from you.

We've added a link to extended documentation, and we made an attempt to fix the pipelines (except for mentioned one). If they succeed now, we believe that everything is ready to merge!

Also, since we expect this is not the last MR we would have, we'd like to ask you several quesitons:

  1. As far as we could see, the commits are not squashed at the MR, so is there any style of commit names that we should follow then?
  2. When we are replying to you, should we also create a review, or it's fine to answer via single message?
stephane-caron commented 4 months ago

Sounds great :smiley:

As far as we could see, the commits are not squashed at the MR, so is there any style of commit names that we should follow then?

We don't enforce a particular style. As long as the commit messages are informative and help navigate the change history (in my opinion yours are :+1:) that's good to go. When the PR is ready we will merge it into main with a merge commit.

When we are replying to you, should we also create a review, or it's fine to answer via single message?

In PRs there are two kinds of discussions: code-related discussions (associated with code) and free discussions (on the PR page). When reviewers read the code they mostly start code-related discussions. The point of code-related discussions is that we want to make sure all of them are marked as "resolved" before merging the PR.

Thus you don't need to create a review when you reply. It is fine to reply separately in each code-related discussion, as you have done. And if there is something else (maybe not code-related) to discuss you can shoot a single message on the PR page separately.

domrachev03 commented 4 months ago

We have tried to fix the pipeline yet again, but it is hard to check documentation build, since we have to wait for your approval each time to see a result. The linting pipeline should be fine.

Do you have any suggestion on a better way of repairing documentation pipeline apart from fixing errors one by one?

stephane-caron commented 4 months ago

Yes, pipelines have to be approved for first-time contributors. After this PR is merged you won't need approval any more for the following ones.

Don't worry about the documentation, I can fix it quickly afterwards. Let me know when everything else is ready, then I will roundup all code-related discussions and move ahead with the merge.

domrachev03 commented 4 months ago

If documentation could be fixed later, then everything else is ready!

stephane-caron commented 4 months ago

Alright, let's move forward. Thank you both for this first contribution! Looking forward to the next barriers :smiley: