From 29a22480f79e1b827da9bd88fb8acccf96ed73c1 Mon Sep 17 00:00:00 2001 From: thanhndv212 Date: Sat, 1 Aug 2026 23:46:33 +0200 Subject: [PATCH 1/2] [manipulation] Fix ConstraintGraphFactory combinatorial blowup from missing negative-memoization MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GraphFactoryAbstract._recurse()'s isNewState check was `not self._existState(nGrasps)`, but self.states (backing _existState) is only populated when graspIsAllowed(nGrasps) is True. So a *rejected* grasp combination was never marked visited, and got its entire descendant subtree redundantly re-explored from every distinct parent path that reached it. The subtree rooted at a given nGrasps is a pure function of its content alone (ngrippers/nhandles are the input lists with a fixed set removed, independent of removal order; depth is likewise path-independent), so re-reaching the same nGrasps via a different gripper-assignment order is always pure wasted recomputation, never new discovery. Add a separate _visitedGrasps set, populated for every nGrasps regardless of accept/reject, independent of self.states (which has a different meaning: actually-created State objects that other code depends on). For 8 grippers x 7 handles (a downstream project's worst-case scene), this caused a 20+ minute hang building the constraint graph for a single phase. Also corrects the class docstring, which described recursion stopping on a rejected grasp set — it doesn't and shouldn't (needed to support non-monotonic filter rules); the docstring just didn't match the code. --- .../manipulation/constraint_graph_factory.py | 34 +++++++++---------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/src/pyhpp/manipulation/constraint_graph_factory.py b/src/pyhpp/manipulation/constraint_graph_factory.py index 14ca630e..22a59377 100644 --- a/src/pyhpp/manipulation/constraint_graph_factory.py +++ b/src/pyhpp/manipulation/constraint_graph_factory.py @@ -249,23 +249,18 @@ class GraphFactoryAbstract(ABC): The first node is defined by the empty set. The graph is built recursiveley as follows: - if the set of grasps defining the node is not allowed (method \\link + For any pair \\f$(g,h)\\f$ of available grippers and available handles, + \\li if the resulting set of grasps is allowed (method \\link constraint_graph_factory.GraphFactoryAbstract.graspIsAllowed - graspIsAllowed \\endlink), return. - - Otherwise, for any pair \\f$(g,h)\f$ of available grippers and available - handles, - \\li build a new state by adding grasp \\f$(g,h)\\f$ to the current set of - grasps (method - \\link constraint_graph_factory.GraphFactoryAbstract.makeState - makeState\\endlink), - \\li build a transition from the current state to the new state (method \\link - constraint_graph_factory.GraphFactoryAbstract.makeTransition - makeTransition \\endlink) - \\li build a loopTransition from the current state to itself (method \\link - constraint_graph_factory.GraphFactoryAbstract.makeLoopTransition - makeLoopTransition \\endlink) - \\li repeat the two above states to the new state. + graspIsAllowed \\endlink), build a new state (method \\link + constraint_graph_factory.GraphFactoryAbstract.makeState + makeState\\endlink) and a transition from the current state (method + \\link constraint_graph_factory.GraphFactoryAbstract.makeTransition + makeTransition \\endlink), + \\li regardless of whether the next set of grasps is allowed, recurse into + it if it has not been visited before (to support non-monotonic filter + rules). A rejected set of grasps is memoized so its subtree is never + re-explored from a different parent path. """ def __init__(self): @@ -283,6 +278,9 @@ def __init__(self): self.states = dict() self.transitions = set() + # Recursion-visited memo for all nGrasps, accepted or rejected. + # Separate from self.states, which only holds created State objects. + self._visitedGrasps = set() # # the handle names self.handles = tuple() # strings # # the gripper names @@ -507,7 +505,9 @@ def _recurse(self, grippers, handles, grasps, depth): nGrasps = grasps[:isg] + (ish,) + grasps[isg + 1 :] nextIsAllowed = self.graspIsAllowed(nGrasps) - isNewState = not self._existState(nGrasps) + isNewState = nGrasps not in self._visitedGrasps + if isNewState: + self._visitedGrasps.add(nGrasps) if nextIsAllowed: nnext = self._makeState(nGrasps, depth + 1) From d94b74ade51ca77dd5c27f8b6b643befb76bea18 Mon Sep 17 00:00:00 2001 From: thanhndv212 Date: Tue, 18 Aug 2026 15:59:27 +0200 Subject: [PATCH 2/2] fix(manipulation): make generate() safe to call more than once _visitedGrasps was only initialized in __init__, so a second generate() call (e.g. after adjusting graspIsAllowed rules) would treat every previously-rejected combo as permanently visited and never re-examine it. Reseed from self.states at the start of generate() so already-built (accepted) subtrees are still skipped, while rejected combos get a fresh chance. Suggested-by: psardin001 --- src/pyhpp/manipulation/constraint_graph_factory.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/pyhpp/manipulation/constraint_graph_factory.py b/src/pyhpp/manipulation/constraint_graph_factory.py index 22a59377..cdb4faf8 100644 --- a/src/pyhpp/manipulation/constraint_graph_factory.py +++ b/src/pyhpp/manipulation/constraint_graph_factory.py @@ -364,6 +364,10 @@ def generate(self): Go through the combinatorial defined by the grippers and handles and create the states and transitions. """ + # Re-seed with already-accepted states so their subtrees aren't + # re-walked, but leave previously-rejected combos unmarked so a + # regenerate after a rule change gets to re-evaluate them. + self._visitedGrasps = set(self.states) grasps = (None,) * len(self.grippers) self._recurse(self.grippers, self.handles, grasps, 0)