Skip to content

Commit 106c138

Browse files
committed
Implement pruning inside serial container
By inefficient inverse lookup. Also add disabled test for the inverted inference (failure inside should prune outside)
1 parent 36d9d3e commit 106c138

File tree

4 files changed

+69
-13
lines changed

4 files changed

+69
-13
lines changed

core/include/moveit/task_constructor/container_p.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,7 @@ class ContainerBasePrivate : public StagePrivate
136136
}
137137

138138
/// copy external_state to a child's interface and remember the link in internal_to map
139+
template <Interface::Direction>
139140
void copyState(Interface::iterator external, const InterfacePtr& target, bool updated);
140141
/// lift solution from internal to external level
141142
void liftSolution(const SolutionBasePtr& solution, const InterfaceState* internal_from,
@@ -207,7 +208,8 @@ class ParallelContainerBasePrivate : public ContainerBasePrivate
207208

208209
private:
209210
/// callback for new externally received states
210-
void onNewExternalState(Interface::Direction dir, Interface::iterator external, bool updated);
211+
template <typename Interface::Direction>
212+
void onNewExternalState(Interface::iterator external, bool updated);
211213
};
212214
PIMPL_FUNCTIONS(ParallelContainerBase)
213215

core/include/moveit/task_constructor/storage.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,9 @@ class InterfaceState
146146
private:
147147
planning_scene::PlanningSceneConstPtr scene_;
148148
PropertyMap properties_;
149+
/// trajectories which are *timewise before* this state
149150
Solutions incoming_trajectories_;
151+
/// trajectories which are *timewise after* this state
150152
Solutions outgoing_trajectories_;
151153

152154
// members needed for priority scheduling in Interface list

core/src/container.cpp

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -106,10 +106,17 @@ void ContainerBasePrivate::compute() {
106106
static_cast<ContainerBase*>(me_)->compute();
107107
}
108108

109+
template <Interface::Direction dir>
109110
void ContainerBasePrivate::copyState(Interface::iterator external, const InterfacePtr& target, bool updated) {
110-
// TODO: update internal's prio from external's new priority
111-
if (updated)
111+
if (updated) {
112+
// TODO(v4hn): This is inefficient, consider storing inverse mapping for each start/end pair in children
113+
// for parallel containers there will be multiple internal states
114+
std::for_each(internal_to_external_.begin(), internal_to_external_.end(), [&external](auto& p) {
115+
if (p.second == &*external)
116+
setStatus<dir>(p.first, external->priority().status());
117+
});
112118
return;
119+
}
113120

114121
// create a clone of external state within target interface (child's starts() or ends())
115122
auto internal = states_.insert(states_.end(), InterfaceState(*external));
@@ -497,8 +504,9 @@ void SerialContainerPrivate::resolveInterface(InterfaceFlags expected) {
497504
validateInterface<START_IF_MASK>(*first.pimpl(), expected);
498505
// connect first child's (start) pull interface
499506
if (const InterfacePtr& target = first.pimpl()->starts())
500-
starts_.reset(new Interface(
501-
[this, target](Interface::iterator it, bool updated) { this->copyState(it, target, updated); }));
507+
starts_.reset(new Interface([this, target](Interface::iterator it, bool updated) {
508+
this->copyState<Interface::FORWARD>(it, target, updated);
509+
}));
502510
} catch (InitStageException& e) {
503511
exceptions.append(e);
504512
}
@@ -521,8 +529,9 @@ void SerialContainerPrivate::resolveInterface(InterfaceFlags expected) {
521529
validateInterface<END_IF_MASK>(*last.pimpl(), expected);
522530
// connect last child's (end) pull interface
523531
if (const InterfacePtr& target = last.pimpl()->ends())
524-
ends_.reset(new Interface(
525-
[this, target](Interface::iterator it, bool updated) { this->copyState(it, target, updated); }));
532+
ends_.reset(new Interface([this, target](Interface::iterator it, bool updated) {
533+
this->copyState<Interface::BACKWARD>(it, target, updated);
534+
}));
526535
} catch (InitStageException& e) {
527536
exceptions.append(e);
528537
}
@@ -622,11 +631,11 @@ void ParallelContainerBasePrivate::resolveInterface(InterfaceFlags expected) {
622631
// States received by the container need to be copied to all children's pull interfaces.
623632
if (expected & READS_START)
624633
starts().reset(new Interface([this](Interface::iterator external, bool updated) {
625-
this->onNewExternalState(Interface::FORWARD, external, updated);
634+
this->onNewExternalState<Interface::FORWARD>(external, updated);
626635
}));
627636
if (expected & READS_END)
628637
ends().reset(new Interface([this](Interface::iterator external, bool updated) {
629-
this->onNewExternalState(Interface::BACKWARD, external, updated);
638+
this->onNewExternalState<Interface::BACKWARD>(external, updated);
630639
}));
631640

632641
required_interface_ = expected;
@@ -663,10 +672,10 @@ void ParallelContainerBasePrivate::validateConnectivity() const {
663672
ContainerBasePrivate::validateConnectivity();
664673
}
665674

666-
void ParallelContainerBasePrivate::onNewExternalState(Interface::Direction dir, Interface::iterator external,
667-
bool updated) {
675+
template <Interface::Direction dir>
676+
void ParallelContainerBasePrivate::onNewExternalState(Interface::iterator external, bool updated) {
668677
for (const Stage::pointer& stage : children())
669-
copyState(external, stage->pimpl()->pullInterface(dir), updated);
678+
copyState<dir>(external, stage->pimpl()->pullInterface(dir), updated);
670679
}
671680

672681
ParallelContainerBase::ParallelContainerBase(ParallelContainerBasePrivate* impl) : ContainerBase(impl) {}

core/test/test_serial.cpp

Lines changed: 44 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,13 @@ void resetIds() {
145145
Connect::id_ = 0;
146146
}
147147

148+
template <typename C, typename S>
149+
auto add(C& container, S* stage) -> S* {
150+
Stage::pointer ptr{ stage };
151+
container.add(std::move(ptr));
152+
return stage;
153+
}
154+
148155
// https://github.com/ros-planning/moveit_task_constructor/issues/182
149156
TEST(ConnectConnect, SuccSucc) {
150157
resetIds();
@@ -208,7 +215,7 @@ TEST(Pruning, PruningMultiForward) {
208215
t.add(Stage::pointer(new GeneratorMockup()));
209216
// spawn two solutions for the only incoming state
210217
t.add(Stage::pointer(new ForwardMockup({ 0, 0 }, 2)));
211-
// fail to exten
218+
// fail to extend the second solution
212219
t.add(Stage::pointer(new ForwardMockup({ 0, inf })));
213220

214221
t.plan();
@@ -268,3 +275,39 @@ TEST(Pruning, ConnectConnectBackward) {
268275
EXPECT_EQ(c2->calls_, 3u);
269276
EXPECT_EQ(c1->calls_, 6u);
270277
}
278+
279+
TEST(Pruning, PropagateInsideContainerBoundaries) {
280+
resetIds();
281+
Task t;
282+
t.setRobotModel(getModel());
283+
add(t, new BackwardMockup({ inf }));
284+
add(t, new GeneratorMockup({ 0 }));
285+
auto c{ std::make_unique<SerialContainer>() };
286+
auto con = add(*c, new Connect());
287+
add(*c, new GeneratorMockup({ 0 }));
288+
t.add(std::move(c));
289+
290+
t.plan();
291+
292+
// the failure in the backward stage (outside the container)
293+
// should prune the expected computation of con
294+
EXPECT_EQ(con->calls_, 0);
295+
}
296+
297+
TEST(Pruning, DISABLED_PropagateOutsideContainerBoundaries) {
298+
resetIds();
299+
Task t;
300+
t.setRobotModel(getModel());
301+
auto back = add(t, new BackwardMockup());
302+
add(t, new BackwardMockup());
303+
add(t, new GeneratorMockup({ 0 }));
304+
auto c{ std::make_unique<SerialContainer>() };
305+
add(*c, new ForwardMockup({ inf }));
306+
add(*c, new ForwardMockup());
307+
t.add(std::move(c));
308+
309+
t.plan();
310+
311+
// the failure inside the container should prune computing of back
312+
EXPECT_EQ(back->calls_, 0);
313+
}

0 commit comments

Comments
 (0)