Skip to content

Use unordered containers in DeMorganSimplifier#2232

Open
esseivaju wants to merge 3 commits intoceleritas-project:developfrom
esseivaju:demorgan-unordered-containers
Open

Use unordered containers in DeMorganSimplifier#2232
esseivaju wants to merge 3 commits intoceleritas-project:developfrom
esseivaju:demorgan-unordered-containers

Conversation

@esseivaju
Copy link
Member

Follow-up to #2231, use map and set instead of pre-allocated vectors.

@esseivaju esseivaju requested a review from amandalund February 5, 2026 18:38
@esseivaju esseivaju added the minor Refactoring or minor internal changes/fixes label Feb 5, 2026
@esseivaju esseivaju requested a review from sethrj as a code owner February 5, 2026 18:38
@esseivaju esseivaju added the orange Work on ORANGE geometry engine label Feb 5, 2026
@codecov
Copy link

codecov bot commented Feb 5, 2026

Codecov Report

❌ Patch coverage is 93.93939% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.82%. Comparing base (43e05ae) to head (092ff67).

Files with missing lines Patch % Lines
src/orange/orangeinp/detail/DeMorganSimplifier.cc 93.93% 0 Missing and 2 partials ⚠️
Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #2232   +/-   ##
========================================
  Coverage    86.81%   86.82%           
========================================
  Files         1330     1330           
  Lines        42019    42027    +8     
  Branches     12950    12949    -1     
========================================
+ Hits         36480    36489    +9     
  Misses        4335     4335           
+ Partials      1204     1203    -1     
Files with missing lines Coverage Δ
src/orange/orangeinp/detail/DeMorganSimplifier.hh 100.00% <ø> (ø)
src/orange/orangeinp/detail/DeMorganSimplifier.cc 97.72% <93.93%> (+0.10%) ⬆️

... and 4 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions
Copy link

github-actions bot commented Feb 5, 2026

Test summary

 5 893 files   9 431 suites   18m 27s ⏱️
 2 146 tests  2 118 ✅  28 💤 0 ❌
32 254 runs  32 096 ✅ 158 💤 0 ❌

Results for commit 092ff67.

♻️ This comment has been updated with latest results.

CELER_ASSERT(node_ids_translation_[negated->node.get()].unmodified);
negated->node
= node_ids_translation_[negated->node.get()].unmodified;
CELER_ASSERT(this->translation(negated->node).unmodified);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the places where we call translation() inside an assertion, is the node always guaranteed to already be in the map, or is it possible for the debug check to create a new entry?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah thanks good catch! There's no guarantee that the node is present when calling the assertion. I've reworked this to avoid the side-effect in the assertion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

minor Refactoring or minor internal changes/fixes orange Work on ORANGE geometry engine

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants