Skip to content

Use unordered containers in DeMorganSimplifier #2232

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.

Sign up for GitHub

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

Open
esseivaju wants to merge 3 commits into
base: develop
Choose a base branch
from
+69 −36
Open

Use unordered containers in DeMorganSimplifier #2232

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d4a504f
replace vectors with map/set
esseivaju Feb 5, 2026
c5cbc2c
avoid assertion side-effect
esseivaju Feb 5, 2026
092ff67
Merge branch 'develop' into demorgan-unordered-containers
esseivaju Feb 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 56 additions & 32 deletions src/orange/orangeinp/detail/DeMorganSimplifier.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,28 @@ NodeId DeMorganSimplifier::MatchingNodes::equivalent_node() const
DeMorganSimplifier::DeMorganSimplifier(CsgTree const& tree)
: tree_(tree), parents_(tree_.size())
{
new_negated_nodes_.resize(tree_.size());
negated_join_nodes_.resize(tree_.size());
node_ids_translation_.resize(tree_.size());
}

//---------------------------------------------------------------------------//
/*!
* Access or create a translation entry.
*/
DeMorganSimplifier::MatchingNodes& DeMorganSimplifier::translation(NodeId id)
{
return node_ids_translation_.try_emplace(id).first->second;
}

//---------------------------------------------------------------------------//
/*!
* Find a translation entry.
*/
DeMorganSimplifier::MatchingNodes const*
DeMorganSimplifier::find_translation(NodeId id) const
{
auto iter = node_ids_translation_.find(id);
if (iter == node_ids_translation_.end())
return nullptr;
return &iter->second;
}

//---------------------------------------------------------------------------//
Expand All @@ -65,10 +84,16 @@ TransformedTree DeMorganSimplifier::operator()()
auto simplified_tree{this->build_simplified_tree()};
std::vector<NodeId> equivalent_nodes;
equivalent_nodes.reserve(tree_.size());
for (auto node_id : range(tree_.size()))
for (auto node_id : range(NodeId{tree_.size()}))
{
equivalent_nodes.push_back(
node_ids_translation_[node_id].equivalent_node());
if (auto const* trans = this->find_translation(node_id))
{
equivalent_nodes.push_back(trans->equivalent_node());
}
else
{
equivalent_nodes.push_back(NodeId{});
}
}
return {simplified_tree, equivalent_nodes};
}
Expand Down Expand Up @@ -107,7 +132,7 @@ void DeMorganSimplifier::find_join_negations()
tree_[this->dealias(negated->node)]))
{
// This is a negated join node
negated_join_nodes_[negated->node.get()] = true;
negated_join_nodes_.insert(negated->node);
this->add_negation_for_operands(negated->node);
}
}
Expand Down Expand Up @@ -148,14 +173,14 @@ void DeMorganSimplifier::add_negation_for_operands(NodeId node_id)
{
// This negated join node has a join operand, so we'll have to
// insert a negated join of that join operand and its operands
negated_join_nodes_[join_operand.get()] = true;
negated_join_nodes_.insert(join_operand);
this->add_negation_for_operands(join_operand);
}
else if (!std::holds_alternative<Negated>(target_node))
{
// Negate each operand unless it's a negated node, in which
// case double negation will cancel to the child of that operand
new_negated_nodes_[join_operand.get()] = true;
new_negated_nodes_.insert(join_operand);
}
}
}
Expand Down Expand Up @@ -192,9 +217,9 @@ CsgTree DeMorganSimplifier::build_simplified_tree()
// We're never inserting a negated node pointing to a
// joined or negated node so it's child must have an
// unmodified equivalent in the simplified tree
CELER_ASSERT(node_ids_translation_[negated->node.get()].unmodified);
negated->node
= node_ids_translation_[negated->node.get()].unmodified;
auto& trans = this->translation(negated->node);
CELER_ASSERT(trans.unmodified);
negated->node = trans.unmodified;
}
else if (auto* joined = std::get_if<Joined>(&new_node))
{
Expand All @@ -205,20 +230,21 @@ CsgTree DeMorganSimplifier::build_simplified_tree()
// That means we should find an equivalent node for
// each operand, either a simplified negated join or an
// unmodified node
CELER_ASSERT(node_ids_translation_[op.get()].equivalent_node());
op = node_ids_translation_[op.get()].equivalent_node();
auto& trans = this->translation(op);
CELER_ASSERT(trans.equivalent_node());
op = trans.equivalent_node();
}
}

auto [new_id, inserted] = result.insert(std::move(new_node));
auto& trans = node_ids_translation_[node_id.get()];
auto& trans = this->translation(node_id);

CELER_ASSERT(!trans.unmodified);
// Record the new node id for parents of that node
trans.unmodified = new_id;

// We might have to insert a negated version of that node
if (new_negated_nodes_[node_id.get()])
if (new_negated_nodes_.count(node_id))
{
Node const& target_node{tree_[this->dealias(node_id)]};
CELER_ASSERT(!std::holds_alternative<Negated>(target_node)
Expand All @@ -237,9 +263,9 @@ CsgTree DeMorganSimplifier::build_simplified_tree()
// new tree.
// This is not always the exact same node, e.g., if the volume
// points to a negated join, it will still be simplified
CELER_ASSERT(node_ids_translation_[volume.get()].equivalent_node());
result.insert_volume(
node_ids_translation_[volume.get()].equivalent_node());
auto& trans = this->translation(volume);
CELER_ASSERT(trans.equivalent_node());
result.insert_volume(trans.equivalent_node());
}

return result;
Expand Down Expand Up @@ -274,10 +300,9 @@ bool DeMorganSimplifier::process_negated_joined_nodes(NodeId node_id,
{
// Redirect parents looking for this node to the new Joined
// node which is logically equivalent
CELER_ASSERT(
node_ids_translation_[negated->node.get()].opposite_join);
node_ids_translation_[node_id.get()].simplified_to
= node_ids_translation_[negated->node.get()].opposite_join;
auto& trans = this->translation(negated->node);
CELER_ASSERT(trans.opposite_join);
this->translation(node_id).simplified_to = trans.opposite_join;
return false;
}

Expand Down Expand Up @@ -318,14 +343,13 @@ bool DeMorganSimplifier::process_negated_joined_nodes(NodeId node_id,
else if (auto const* joined = std::get_if<Joined>(target_node))
{
// Check if this node needs a simplification
if (negated_join_nodes_[node_id.get()])
if (negated_join_nodes_.count(node_id))
{
// Insert the negated node
auto [new_id, inserted]
= result.insert(this->build_negated_node(*joined));
// Record that we inserted an opposite join for that node
node_ids_translation_[node_id.get()].opposite_join
= std::move(new_id);
this->translation(node_id).opposite_join = std::move(new_id);
}
return this->should_insert_join(node_id);
}
Expand All @@ -340,7 +364,7 @@ bool DeMorganSimplifier::process_negated_joined_nodes(NodeId node_id,
*
* \return Join node with opposite operation and negated operands
*/
Joined DeMorganSimplifier::build_negated_node(Joined const& joined) const
Joined DeMorganSimplifier::build_negated_node(Joined const& joined)
{
// Insert the opposite join
auto const& [op, nodes] = joined;
Expand All @@ -357,17 +381,17 @@ Joined DeMorganSimplifier::build_negated_node(Joined const& joined) const
{
// We should have recorded that this node was necessary
// for a join
CELER_ASSERT(node_ids_translation_[neg->node.get()].unmodified);
operands.push_back(
node_ids_translation_[neg->node.get()].unmodified);
auto& trans = this->translation(neg->node);
CELER_ASSERT(trans.unmodified);
operands.push_back(trans.unmodified);
}
else
{
// Otherwise, we should have inserted a negated
// version of that operand in the simplified tree.
// It's either a simplified join or a negated node
operands.push_back([&] {
auto& trans = node_ids_translation_[n.get()];
auto& trans = this->translation(n);
CELER_ASSERT(trans.new_negation || trans.opposite_join);
if (trans.new_negation)
return trans.new_negation;
Expand Down Expand Up @@ -406,7 +430,7 @@ bool DeMorganSimplifier::should_insert_join(NodeId node_id)
auto has_negated_join_parent = [&](NodeId n) {
for (auto p : range(first_node_id_, NodeId{tree_.size()}))
{
if (parents_.count({n, p}) && negated_join_nodes_[p.get()])
if (parents_.count({n, p}) && negated_join_nodes_.count(p))
return true;
}
return false;
Expand Down
17 changes: 13 additions & 4 deletions src/orange/orangeinp/detail/DeMorganSimplifier.hh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
//---------------------------------------------------------------------------//
#pragma once

#include <unordered_map>
#include <unordered_set>
#include <utility>
#include <vector>
Expand Down Expand Up @@ -64,6 +65,7 @@ class DeMorganSimplifier
};

using SparseMatrix2D = std::unordered_set<NodePair, NodePairHash>;
using NodeSet = std::unordered_set<NodeId>;

//! CsgTree node 0 is always True{} and can't be the parent of any node
//! so reuse that bit to tell that a given node is a volume
Expand Down Expand Up @@ -104,6 +106,8 @@ class DeMorganSimplifier
NodeId equivalent_node() const;
};

using NodeMap = std::unordered_map<NodeId, MatchingNodes>;

// Dereference Aliased nodes
NodeId dealias(NodeId) const;

Expand All @@ -120,27 +124,32 @@ class DeMorganSimplifier
bool process_negated_joined_nodes(NodeId, CsgTree&);

// Create an opposite Joined node
Joined build_negated_node(Joined const&) const;
Joined build_negated_node(Joined const&);

// Check if this join node should be inserted in the simplified tree
bool should_insert_join(NodeId);

// Access or create a translation entry
MatchingNodes& translation(NodeId);
// Find a translation entry, or nullptr if none exist
MatchingNodes const* find_translation(NodeId) const;

//! the tree to simplify
CsgTree const& tree_;

//! Set when we must insert a \c Negated parent for the given index
std::vector<bool> new_negated_nodes_;
NodeSet new_negated_nodes_;

//! Set when \c Joined nodes have a \c Negated parent, so we need to insert
//! an opposite join node with negated operands
std::vector<bool> negated_join_nodes_;
NodeSet negated_join_nodes_;

//! Parents matrix. If the pair {e1, e2} exists, e2 is parent of e1
SparseMatrix2D parents_;

//! Used during construction of the simplified tree to map replaced nodes
//! in the original tree to their new id in the simplified tree
std::vector<MatchingNodes> node_ids_translation_;
NodeMap node_ids_translation_;
};

//---------------------------------------------------------------------------//
Expand Down
Loading