WIP: Fix clone and restore for invalid non colliding cache #1455
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 29 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.
…ating the vector.
…CollidingGrabbedGrabberLinkPairsWhenGrabbed in CheckSelfCollisionChecking
…nkCollisions==false. also share the utilities for CollisioReport update and printing.
…' to clarify meaning.
…fixGrabbedSelfColCheckAsym
…fixGrabbedSelfColCheckAsym
…fixGrabbedSelfColCheckAsym
…fixGrabbedSelfColCheckAsym
…fixGrabbedSelfColCheckAsym
…inbodystatesaver.cpp.
- 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.
… reviveOrderOfGrabbed
… reviveOrderOfGrabbed
- Issue - When the new Grabbed instance is created in saver or Clone, the _pGrabbedSaver and _pGrabberSaver of the new Grabbed are different from those of the original Grabbed. - Thus, if isListNonCollidingLinksValid=false, wrong state was used in ComputeListNonCollidingLinks. - Resolution - Copy the consistent states from the original Grabbed to the new Grabbed. - To do so, introduce the constructor of Grabbed to copy the saver states from the original Grabbed. In addition, introduce the constructor of StateSavers to copy the states from the original savers. - If the states are not copy-able, throw exception. Also, this API is very advanced and not expected to be used from the usual users. Thus, make such functionality as the constructor.
…deterministic grabbed self collision checking.
…oneAndRestoreForInvalidNonColCache
…GrabberSaver of Grabbed instances when cloning or restoring, to avoid incorrect computation of the non colliding list cache.
… reviveOrderOfGrabbed
…oneAndRestoreForInvalidNonColCache
… reviveOrderOfGrabbed
…oneAndRestoreForInvalidNonColCache
…Transformations is initialized.
…grabbed-grabber link pairs. : #1438 (comment)
Co-authored-by: Puttichai Lertkultanon <[email protected]>
Co-authored-by: Puttichai Lertkultanon <[email protected]>
Co-authored-by: Puttichai Lertkultanon <[email protected]>
…rabbed Co-authored-by: Puttichai Lertkultanon <[email protected]>
…include/openrave/kinbody.h
…oneAndRestoreForInvalidNonColCache
…gelog.rst, CMakeLists.txt
… reviveOrderOfGrabbed
…oneAndRestoreForInvalidNonColCache
…abled, since it's relatively noisy.
… reviveOrderOfGrabbed
…oneAndRestoreForInvalidNonColCache
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
summary
Issue
_isListNonCollidingLinksValidis false, andCloneor kinbodysaver's restoring is called,Grabbed's internal savers are inconsistent.Grabbedhas_pGrabbedSaverand_pGrabberSaverat the configurationA. Later, set the configuration toB. Then, callCloneorKinBodyStateSaver::Restore(pbody). In such case, the newGrabbedinstance is created for the cloned/restored body. But, the newly createdGrabbedhas_pGrabbedSaverand_pGrabberSaverwith the configurationB. Therefore, the derivedlistNonCollidingLinksare different between the original one and the new one.Resolution
Grabbedto copy the saver's states from otherGrabbed.KinBodyStateSaver, andRobotStateSaverto copy the states from other savers.Saveroptions. It only supports the copy-able states. If the state is pointer, the copying of it might not be always expected.