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

WIP: Revive order of grabbed #1450

Open
wants to merge 40 commits into
base: production
Choose a base branch
from
Open

WIP: Revive order of grabbed #1450

wants to merge 40 commits into from

Conversation

snozawa
Copy link
Collaborator

@snozawa snozawa commented Oct 19, 2024

Summary

Issue

Resolution

  • Since unodered_map is introduced for the performance improvement, it's not good idea to revive other STL containers for the representation.
  • Instead, while keeping the unordered_map, maintain the order by adding the unique id into Grabbed class. Such unique id is (mostly) monotonically increasing and represents the order when grabbed.
  • In addition, Grabbed::ComputeListNonCollidingLinks will consider the order when grabbed via the unique id. If other grabbed body has larger unique id, it means it's grabbed later and don't need to compute the listNonCollidingLinks.
  • Overall, this PR tries to make the result of lazy-computation equivalent as the result of non-lazy-computation.

Shunichi Nozawa added 21 commits September 25, 2024 11:58
- Issue
  - This commit tries to improve the reported issue1, issue2, and issue3 : #1436
  - Previously, _listNonCollidingLinksWhenGrabbed is asymmetric : the results obtained from grabbedBody1 is not same as the results obtained from grabbedBody2.
  - However, many codes in CheckSelfCollision assumes it's symmetric. The assumption was broken. In addition, such breakage was amplified after #1421.
- Resolution
  - Instead of store the target link like the previous _listNonCollidingLinksWhenGrabbed_, store the information about the link pairs. This is more accurate and the same methodologies as non-adjacent-links in self colision checking.
  - Separate grabbed-grabber link pair and inter-grabbed link pair.
     - grabbed-grabber link pair, e.g. object-robot link pair, still exists in the Grabbed class as before.
     - inter-grabbed link pair now exists in the KinBody class, since if it's held in Grabbed, it becomes inconsistent and asymmetric.
  - Following the same methodologies in #1421, inter-grabbed link pairs are stored as unordered_map, and its key is combined env body indices of two grabbed bodies.
…CollidingGrabbedGrabberLinkPairsWhenGrabbed in CheckSelfCollisionChecking
…nkCollisions==false. also share the utilities for CollisioReport update and printing.
- Issue
  - Previously, KinBody remembers the order when the grabbed bodies are grabbed.
  - However, it's broken since we introduced unordered_map. Especially, listNonCollidingLinksWhenGrabbed computation becomes less deterministic along with its lazy computatoin.
- Resolution
  - Since unordered_map is introduced to improve the performance, not good idea to revive std::vector as its representation.
  - Instead, introduce the unique id to remember the order when grabbed.
… it means pOtherGrabbed is grabbed later than 'this'. Thus, pOtherGrabbed did not exist when 'this' was grabbed.

The results of lazy computation should be same as the result of non-lazy computation. Therefore, we should not compute the inter-grabbed collision checking with pOtherGrabbed.
This resolves Issue4 in #1436.
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.

1 participant