Skip to content

Commit

Permalink
Use remove/tryRemove on cuts depending on situation.
Browse files Browse the repository at this point in the history 
Distinguish between remove, which should definitely have the object to
be removed in the result set, and tryRemove, which tests if it is
there, removes it if it is, and returns whether it was removed. This
fixes a crashing assert since in some cases we were asserting that the
object was in the results when we were only trying to remove it if it
was.
  • Loading branch information
@ewencp
ewencp committed Mar 4, 2013
1 parent 773bdc0 commit 09a191c
Show file tree
Hide file tree
Showing 4 changed files with 83 additions and 29 deletions.
24 changes: 19 additions & 5 deletions libprox/include/prox/geom/LevelQueryHandler.hpp
View file
Expand Up @@ -656,23 +656,31 @@ class LevelQueryHandler : public QueryHandler<SimulationTraits> {
//assert(!cnode->active_result);
results.insert(objid);
}
size_t removeResult(CutNodeType* cnode) {
assert(inResultsSlow(cnode->rtnode->aggregateID()));
assert(cnode->active_result);
size_t tryRemoveResult(CutNodeType* cnode) {
cnode->active_result = false;
return results.erase(cnode->rtnode->aggregateID());
}
size_t removeResult(CutNodeType* cnode, const ObjectID& objid) {
size_t tryRemoveResult(CutNodeType* cnode, const ObjectID& objid) {
// Verify with assert, but just trust that they gave us a
// real object child
assert((cnode->rtnode->aggregateID() != objid));
assert(cnode->rtnode->objectChildren());
assert(inResultsSlow(objid));
// Would be nice but we need to guarantee some ordering of
//remove/add, and depends on aggs vs. no aggs
//assert(cnode->active_result);
return results.erase(objid);
}
void removeResult(CutNodeType* cnode) {
assert(inResultsSlow(cnode->rtnode->aggregateID()));
assert(cnode->active_result);
size_t removed = tryRemoveResult(cnode);
assert(removed);
}
void removeResult(CutNodeType* cnode, const ObjectID& objid) {
assert(inResultsSlow(objid));
size_t removed = tryRemoveResult(cnode, objid);
assert(removed);
}
bool inResults(CutNodeType* cnode) const {
assert( (results.find(cnode->rtnode->aggregateID()) != results.end()) == cnode->active_result);
return cnode->active_result;
Expand All @@ -689,6 +697,12 @@ class LevelQueryHandler : public QueryHandler<SimulationTraits> {
assert((results.find(objid) == results.end()));
return false;
}
else if (withAggregates()) {
// However, with aggregates this can be fast again
// because we should have the child in the results
assert((results.find(objid) != results.end()));
return true;
}
return inResultsSlow(objid);
}
bool inResultsSlow(const ObjectID& objid) const {
Expand Down
28 changes: 22 additions & 6 deletions libprox/include/prox/geom/RTreeCutQueryHandler.hpp
View file
Expand Up @@ -688,23 +688,31 @@ class RTreeCutQueryHandler : public QueryHandler<SimulationTraits> {
//assert(!cnode->active_result);
results.insert(objid);
}
size_t removeResult(CutNodeType* cnode) {
assert(inResultsSlow(cnode->rtnode->aggregateID()));
assert(cnode->active_result);
size_t tryRemoveResult(CutNodeType* cnode) {
cnode->active_result = false;
return results.erase(cnode->rtnode->aggregateID());
}
size_t removeResult(CutNodeType* cnode, const ObjectID& objid) {
size_t tryRemoveResult(CutNodeType* cnode, const ObjectID& objid) {
// Verify with assert, but just trust that they gave us a
// real object child
assert((cnode->rtnode->aggregateID() != objid));
assert(cnode->rtnode->objectChildren());
assert(inResultsSlow(objid));
// Would be nice but we need to guarantee some ordering of
//remove/add, and depends on aggs vs. no aggs
//assert(cnode->active_result);
return results.erase(objid);
}
void removeResult(CutNodeType* cnode) {
assert(inResultsSlow(cnode->rtnode->aggregateID()));
assert(cnode->active_result);
size_t removed = tryRemoveResult(cnode);
assert(removed);
}
void removeResult(CutNodeType* cnode, const ObjectID& objid) {
assert(inResultsSlow(objid));
size_t removed = tryRemoveResult(cnode, objid);
assert(removed);
}
bool inResults(CutNodeType* cnode) const {
assert( (results.find(cnode->rtnode->aggregateID()) != results.end()) == cnode->active_result);
return cnode->active_result;
Expand All @@ -721,7 +729,15 @@ class RTreeCutQueryHandler : public QueryHandler<SimulationTraits> {
assert((results.find(objid) == results.end()));
return false;
}
return inResultsSlow(objid);
else if (withAggregates()) {
// However, with aggregates this can be fast again
// because we should have the child in the results
assert((results.find(objid) != results.end()));
return true;
}
else {
return inResultsSlow(objid);
}
}
bool inResultsSlow(const ObjectID& objid) const {
return results.find(objid) != results.end();
Expand Down
26 changes: 20 additions & 6 deletions libprox/include/prox/manual/RTreeManualQueryHandler.hpp
View file
Expand Up @@ -563,23 +563,31 @@ class RTreeManualQueryHandler : public ManualQueryHandler<SimulationTraits> {
//assert(!cnode->active_result);
results.insert(objid);
}
size_t removeResult(CutNodeType* cnode) {
assert(inResultsSlow(cnode->rtnode->aggregateID()));
assert(cnode->active_result);
size_t tryRemoveResult(CutNodeType* cnode) {
cnode->active_result = false;
return results.erase(cnode->rtnode->aggregateID());
}
size_t removeResult(CutNodeType* cnode, const ObjectID& objid) {
size_t tryRemoveResult(CutNodeType* cnode, const ObjectID& objid) {
// Verify with assert, but just trust that they gave us a
// real object child
assert(cnode->rtnode->aggregateID() != objid);
assert((cnode->rtnode->aggregateID() != objid));
assert(cnode->rtnode->objectChildren());
assert(inResultsSlow(objid));
// Would be nice but we need to guarantee some ordering of
//remove/add, and depends on aggs vs. no aggs
//assert(cnode->active_result);
return results.erase(objid);
}
void removeResult(CutNodeType* cnode) {
assert(inResultsSlow(cnode->rtnode->aggregateID()));
assert(cnode->active_result);
size_t removed = tryRemoveResult(cnode);
assert(removed);
}
void removeResult(CutNodeType* cnode, const ObjectID& objid) {
assert(inResultsSlow(objid));
size_t removed = tryRemoveResult(cnode, objid);
assert(removed);
}
bool inResults(CutNodeType* cnode) const {
assert( (results.find(cnode->rtnode->aggregateID()) != results.end()) == cnode->active_result);
return cnode->active_result;
Expand All @@ -596,6 +604,12 @@ class RTreeManualQueryHandler : public ManualQueryHandler<SimulationTraits> {
assert((results.find(objid) == results.end()));
return false;
}
else if (withAggregates()) {
// However, with aggregates this can be fast again
// because we should have the child in the results
assert((results.find(objid) != results.end()));
return true;
}
return inResultsSlow(objid);
}
bool inResultsSlow(const ObjectID& objid) const {
Expand Down
34 changes: 22 additions & 12 deletions libprox/include/prox/rtree/Cut.hpp
View file
Expand Up @@ -101,8 +101,10 @@ class CutBase {
// object ID is picked up automatically for internal nodes.
// void addResult(CutNodeType* cnode);
// void addResult(CutNodeType* cnode, const ObjectID& objid);
// size_t removeResult(CutNodeType* cnode);
// size_t removeResult(CutNodeType* cnode, const ObjectID& objid);
// void removeResult(CutNodeType* cnode);
// void removeResult(CutNodeType* cnode, const ObjectID& objid);
// size_t tryRemoveResult(CutNodeType* cnode);
// size_t tryRemoveResult(CutNodeType* cnode, const ObjectID& objid);
// bool inResults(CutNodeType* cnode) const;
// bool inResults(CutNodeType* cnode, const ObjectID& objid) const;
// bool inResultsSlow(const ObjectID& objid) const;
Expand Down Expand Up @@ -157,12 +159,18 @@ class CutBase {
void addToResults(CutNodeType* cnode, const ObjectID& objid) {
getNativeThis()->addResult(cnode, objid);
}
size_t removeFromResults(CutNodeType* cnode) {
void removeFromResults(CutNodeType* cnode) {
return getNativeThis()->removeResult(cnode);
}
size_t removeFromResults(CutNodeType* cnode, const ObjectID& objid) {
void removeFromResults(CutNodeType* cnode, const ObjectID& objid) {
return getNativeThis()->removeResult(cnode, objid);
}
size_t tryRemoveFromResults(CutNodeType* cnode) {
return getNativeThis()->tryRemoveResult(cnode);
}
size_t tryRemoveFromResults(CutNodeType* cnode, const ObjectID& objid) {
return getNativeThis()->tryRemoveResult(cnode, objid);
}
bool isInResults(CutNodeType* cnode) const {
return getNativeThis()->inResults(cnode);
}
Expand Down Expand Up @@ -633,7 +641,10 @@ class CutBase {
if (!parent_in_results) {
// Just add the child
ObjectID child_id = getLocCache()->iteratorID(objit);
assert(!isInResults(cnode, child_id));
// Note that this is the "slow" check because we need
// to really verify it's absence, not rely on it's
// parent's indicator since it's a new node
assert(!isInResultsSlow(/*cnode,*/ child_id));

addToResults(cnode, child_id);

Expand Down Expand Up @@ -1025,7 +1036,7 @@ class CutBase {
if (node->objectChildren()) {
for(int leaf_idx = 0; leaf_idx < node->size(); leaf_idx++) {
ObjectID leaf_id = getLocCache()->iteratorID(node->object(leaf_idx).object);
size_t leaf_removed = removeFromResults(cnode, leaf_id);
size_t leaf_removed = tryRemoveFromResults(cnode, leaf_id);
if (leaf_removed > 0) {
if (includeRemoval(Change_Coarsened))
destroyEvent.addRemoval( typename QueryEventType::Removal(leaf_id, QueryEventType::Transient) );
Expand Down Expand Up @@ -1227,7 +1238,7 @@ class CutBase {
// events (e.g., because here if we have the children in the
// results, we need to remove them *as well as* the aggregate
// itself).
size_t nremoved = removeFromResults(node);
size_t nremoved = tryRemoveFromResults(node);
// If necessary, make sure children removal is entered into the
// event first
if (nremoved == 0) {
Expand Down Expand Up @@ -1304,7 +1315,7 @@ class CutBase {
// it is the same as the total number of children
for(int leafidx = 0; leafidx < node->size(); leafidx++) {
ObjectID leaf_id = getLocCache()->iteratorID(node->object(leafidx).object);
size_t n_leaf_removed = removeFromResults(cnode, leaf_id);
size_t n_leaf_removed = tryRemoveFromResults(cnode, leaf_id);
if (n_leaf_removed > 0) {
if (includeRemoval(Change_Coarsened))
qevt_out->addRemoval( typename QueryEventType::Removal(leaf_id, QueryEventType::Transient) );
Expand Down Expand Up @@ -1349,7 +1360,7 @@ class CutBase {
bool aggregate_was_in_results = false;
// Only try to remove the child node from results for aggregates
if (usesAggregates()) {
size_t nremoved = removeFromResults(child_cn);
size_t nremoved = tryRemoveFromResults(child_cn);
if (nremoved > 0) {
aggregate_was_in_results = true;
if (includeRemoval(Change_Coarsened))
Expand All @@ -1369,7 +1380,7 @@ class CutBase {
// it is the same as the total number of children
for(int leafidx = 0; leafidx < child_rtnode->size(); leafidx++) {
ObjectID leaf_id = getLocCache()->iteratorID(child_rtnode->object(leafidx).object);
size_t n_leaf_removed = removeFromResults(child_cn, leaf_id);
size_t n_leaf_removed = tryRemoveFromResults(child_cn, leaf_id);
if (n_leaf_removed > 0) {
if (includeRemoval(Change_Coarsened))
qevt_out->addRemoval( typename QueryEventType::Removal(leaf_id, QueryEventType::Transient) );
Expand Down Expand Up @@ -1414,8 +1425,7 @@ class CutBase {
// is breaking, even though I can't see how
//result_it could ever be invalid. Instead, do
//it the hard way and assert:
size_t nremoved = removeFromResults(cnode);
assert(nremoved == 1);
removeFromResults(cnode);
if (includeRemoval(Change_Refined))
evt.addRemoval( typename QueryEventType::Removal(cnode->rtnode->aggregateID(), QueryEventType::Transient) );
events.push_back(evt);
Expand Down

0 comments on commit 09a191c

Please sign in to comment.