Skip to content

Conversation

Drewniok
Copy link
Collaborator

Description

This PR introduces a minor refactor of QuickSim. The key update involves removing the hardcoded number 1.5 in the upper limit calculation, which was originally implemented to accelerate the simulation of SiDB gates. While this approach proved effective in most scenarios, it could lead to rare edge cases causing unexpected behavior. To enhance reliability, this adjustment eliminates the magic number. Importantly, the overall performance remains significantly improved compared to the results presented in QuickSim: Efficient and Accurate Physical Simulation of Silicon Dangling Bond Logic (https://ieeexplore.ieee.org/document/10231266).

Checklist:

  • The pull request only contains commits that are related to it.
  • I have added appropriate tests and documentation.
  • I have added a changelog entry.
  • I have created/adjusted the Python bindings for any new or updated functionality.
  • I have made sure that all CI jobs on GitHub pass.
  • The pull request introduces no new warnings and follows the project's style guidelines.

@Drewniok Drewniok added the enhancement New feature or request label Feb 25, 2025
@Drewniok Drewniok self-assigned this Feb 25, 2025
@Drewniok Drewniok changed the title 🎨 Refactor QuickSim to remove magic number for upper limit calculation 🎨 refactored QuickSim to remove magic number for upper limit calculation Feb 25, 2025
@Drewniok Drewniok changed the title 🎨 refactored QuickSim to remove magic number for upper limit calculation 🎨 Refactored QuickSim to remove magic number for upper limit calculation Feb 25, 2025
@Drewniok Drewniok requested a review from marcelwa February 25, 2025 10:15
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

Copy link

codecov bot commented Feb 25, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.40%. Comparing base (fb71178) to head (500b5f3).
Report is 75 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #679      +/-   ##
==========================================
- Coverage   98.41%   98.40%   -0.01%     
==========================================
  Files         254      254              
  Lines       41149    41167      +18     
  Branches     1869     1871       +2     
==========================================
+ Hits        40495    40511      +16     
- Misses        654      656       +2     
Files with missing lines Coverage Δ
...ion/algorithms/simulation/sidb/clustercomplete.hpp 99.27% <100.00%> (ø)
.../fiction/algorithms/simulation/sidb/quickexact.hpp 100.00% <100.00%> (ø)
...de/fiction/algorithms/simulation/sidb/quicksim.hpp 98.68% <100.00%> (+0.01%) ⬆️
...fiction/technology/charge_distribution_surface.hpp 99.84% <100.00%> (-0.01%) ⬇️
test/algorithms/simulation/sidb/quicksim.cpp 100.00% <100.00%> (ø)
test/technology/charge_distribution_surface.cpp 100.00% <100.00%> (ø)

... and 3 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e8fc98c...500b5f3. Read the comment docs.

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

@Drewniok Drewniok requested a review from marcelwa February 25, 2025 17:07
@Drewniok Drewniok merged commit ac42d89 into cda-tum:main Feb 26, 2025
49 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants