🐛 Fixed bug in three-state simulation and duplicate cds results for `QuickExact`. #388

Drewniok · 2024-03-01T21:39:44Z

Description

This PR fixes incorrect simulation results (issue #385) for some layouts in three-state simulation and some duplicate CDs as results (issue #387).

@wlambooy Thanks again for spotting this!

Checklist:

The pull request only contains commits that are related to it.
I have added appropriate tests and documentation.
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.

… layouts).

github-actions · 2024-03-01T21:50:00Z

clang-tidy review says "All clean, LGTM! 👍"

codecov · 2024-03-01T22:03:34Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.09%. Comparing base (05a1eec) to head (23eeda2).
Report is 8 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff            @@
##             main     #388    +/-   ##
========================================
  Coverage   96.09%   96.09%            
========================================
  Files         102      104     +2     
  Lines       10133    10315   +182     
========================================
+ Hits         9737     9912   +175     
- Misses        396      403     +7

Files	Coverage Δ
...n/algorithms/physical_design/design_sidb_gates.hpp	`96.34% <100.00%> (-0.60%)`	⬇️
.../sidb/check_simulation_results_for_equivalence.hpp	`100.00% <100.00%> (ø)`
.../fiction/algorithms/simulation/sidb/quickexact.hpp	`98.90% <100.00%> (+0.01%)`	⬆️
...fiction/technology/charge_distribution_surface.hpp	`99.41% <100.00%> (+<0.01%)`	⬆️
include/fiction/utils/layout_utils.hpp	`98.51% <ø> (ø)`
include/fiction/utils/math_utils.hpp	`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 26ad67f...23eeda2. Read the comment docs.

… update update_charge_state_of_dependent_cell function.

…es on n positions.

github-actions

clang-tidy made some suggestions

include/fiction/utils/math_utils.hpp

github-actions · 2024-03-04T07:55:14Z

clang-tidy review says "All clean, LGTM! 👍"

github-actions · 2024-03-04T08:18:32Z

clang-tidy review says "All clean, LGTM! 👍"

Drewniok · 2024-03-04T09:20:17Z

New runtimes:

benchmark name                       samples       iterations    est run time
                                     mean          low mean      high mean
                                     std dev       low std dev   high std dev
-------------------------------------------------------------------------------
QuickExact                                     100             1     1.69339 m 
                                         1.03102 s       1.028 s     1.03517 s 
                                        17.8754 ms    14.3439 ms    27.4865 ms 
                                                                               
QuickSim                                       100             1    437.706 ms 
                                        4.32637 ms    4.29823 ms    4.35893 ms 
                                        154.258 us    132.162 us    194.857 us

Old (main-branch):

benchmark name                       samples       iterations    est run time
                                     mean          low mean      high mean
                                     std dev       low std dev   high std dev
-------------------------------------------------------------------------------
QuickExact                                     100             1     1.71701 m 
                                         1.03489 s     1.03412 s     1.03594 s 
                                        4.55122 ms    3.44857 ms    6.61014 ms 
                                                                               
QuickSim                                       100             1    430.069 ms 
                                        4.33497 ms     4.3084 ms    4.36221 ms 
                                        136.425 us    119.873 us    158.676 us

marcelwa

Many thanks for getting at it so quickly!

include/fiction/algorithms/simulation/sidb/check_equivalence_simulation_result.hpp

include/fiction/utils/math_utils.hpp

experiments/equivalence_checking_exact_simulation/equivalence_checking_exact_simulation.cpp

marcelwa · 2024-03-04T10:43:49Z

New runtimes:

benchmark name                       samples       iterations    est run time
                                     mean          low mean      high mean
                                     std dev       low std dev   high std dev
-------------------------------------------------------------------------------
QuickExact                                     100             1     1.69339 m 
                                         1.03102 s       1.028 s     1.03517 s 
                                        17.8754 ms    14.3439 ms    27.4865 ms 
                                                                               
QuickSim                                       100             1    437.706 ms 
                                        4.32637 ms    4.29823 ms    4.35893 ms 
                                        154.258 us    132.162 us    194.857 us

Old (main-branch):

benchmark name                       samples       iterations    est run time
                                     mean          low mean      high mean
                                     std dev       low std dev   high std dev
-------------------------------------------------------------------------------
QuickExact                                     100             1     1.71701 m 
                                         1.03489 s     1.03412 s     1.03594 s 
                                        4.55122 ms    3.44857 ms    6.61014 ms 
                                                                               
QuickSim                                       100             1    430.069 ms 
                                        4.33497 ms     4.3084 ms    4.36221 ms 
                                        136.425 us    119.873 us    158.676 us

Will you put them as a comment in the respective benchmark file?

github-actions · 2024-03-04T10:45:57Z

clang-tidy review says "All clean, LGTM! 👍"

github-actions · 2024-03-04T13:57:00Z

clang-tidy review says "All clean, LGTM! 👍"

github-actions · 2024-03-04T15:11:11Z

clang-tidy review says "All clean, LGTM! 👍"

github-actions · 2024-03-04T16:19:43Z

clang-tidy review says "All clean, LGTM! 👍"

github-actions · 2024-03-05T08:01:10Z

clang-tidy review says "All clean, LGTM! 👍"

experiments/equivalence_checking_exact_simulation/equivalence_checking_exact_simulation.cpp

include/fiction/algorithms/simulation/sidb/check_simulation_results_for_equivalence.hpp

test/algorithms/simulation/sidb/quickexact.cpp

test/benchmark/simulation.cpp

github-actions · 2024-03-05T10:42:58Z

clang-tidy review says "All clean, LGTM! 👍"

include/fiction/algorithms/simulation/sidb/check_simulation_results_for_equivalence.hpp

github-actions · 2024-03-07T07:32:41Z

clang-tidy review says "All clean, LGTM! 👍"

marcelwa

Many thanks!!

Drewniok added 3 commits March 1, 2024 22:33

🐛 if charge index is zweo, all SiDBs are negatively charged (for both…

2273665

… layouts).

🐛 wrong flag.

5db7c78

✅ add unit test to cover more cases.

dfd9f38

Drewniok changed the title ~~🐛 Fixed bug in three-state simulation and duplicate cds results for ``QuickExact''.~~ 🐛 Fixed bug in three-state simulation and duplicate cds results for QuickExact. Mar 1, 2024

Drewniok self-assigned this Mar 1, 2024

Drewniok added the bug Something isn't working label Mar 1, 2024

Drewniok added 2 commits March 1, 2024 22:40

✅ reactivate test.

795c730

🎨 small change.

5845bd8

Drewniok added 9 commits March 3, 2024 20:28

🎨 update validity function to avoid gaps in the definition. Moreover,…

dfaca69

… update update_charge_state_of_dependent_cell function.

✨ new function to to check the equivalence of two simulations results.

bfcd6da

🎨 determine all distributions in separate header file.

33f3092

✅ add experiment to check equality for all layouts in spanned area.

f37328d

🎨 add missing headers.

b9cd4d0

🎨 add function to determine all combinations of distributing k entiti…

a7b67a0

…es on n positions.

🐛 update charge index.

10f4e48

✅ add new tests that failed before.

ed4560b

📝 update docu.

e009293

github-actions bot reviewed Mar 3, 2024

View reviewed changes

include/fiction/utils/math_utils.hpp Outdated Show resolved Hide resolved

Drewniok added 5 commits March 4, 2024 07:16

🎨 add inline keyword.

7b3d0ab

✅ add unit test.

7cd8caa

✅ add unit test.

85c950a

📝 add docu.

024177e

🎨 small change for headers.

e183f3b

Drewniok requested a review from marcelwa March 4, 2024 07:39

🎨 small change.

6a2c1ea

🎨 small change.

734c58e

marcelwa requested changes Mar 4, 2024

View reviewed changes

🎨 implement Marcel's feedback.

4046aca

Drewniok requested a review from marcelwa March 4, 2024 13:42

🎨 small modification.

6d42c4d

✅ add more unit tests.

046adce

✅ add more unit tests.

02e0cb4

marcelwa requested changes Mar 5, 2024

View reviewed changes

🎨 implement Marcel's feedback.

346bb1f

Drewniok requested a review from marcelwa March 5, 2024 10:32

marcelwa reviewed Mar 5, 2024

View reviewed changes

include/fiction/algorithms/simulation/sidb/check_simulation_results_for_equivalence.hpp Outdated Show resolved Hide resolved

This was linked to issues Mar 5, 2024

🐛 21 Physically valid layouts (for the same layout) not detected by QuickExact #385

Closed

🐛 QuickExact returns duplicate charge configurations #387

Closed

🎨 implement Marcel's feedback.

23eeda2

Drewniok requested a review from marcelwa March 7, 2024 07:23

marcelwa approved these changes Mar 7, 2024

View reviewed changes

marcelwa merged commit aecd480 into cda-tum:main Mar 7, 2024
62 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🐛 Fixed bug in three-state simulation and duplicate cds results for `QuickExact`. #388

🐛 Fixed bug in three-state simulation and duplicate cds results for `QuickExact`. #388

Drewniok commented Mar 1, 2024 •

edited

Loading

github-actions bot commented Mar 1, 2024

codecov bot commented Mar 1, 2024 •

edited

Loading

github-actions bot left a comment

github-actions bot commented Mar 4, 2024

github-actions bot commented Mar 4, 2024

Drewniok commented Mar 4, 2024

marcelwa left a comment

marcelwa commented Mar 4, 2024

github-actions bot commented Mar 4, 2024

github-actions bot commented Mar 4, 2024

github-actions bot commented Mar 4, 2024

github-actions bot commented Mar 4, 2024

github-actions bot commented Mar 5, 2024

github-actions bot commented Mar 5, 2024

github-actions bot commented Mar 7, 2024

marcelwa left a comment

🐛 Fixed bug in three-state simulation and duplicate cds results for QuickExact. #388

🐛 Fixed bug in three-state simulation and duplicate cds results for QuickExact. #388

Conversation

Drewniok commented Mar 1, 2024 • edited Loading

Description

Checklist:

github-actions bot commented Mar 1, 2024

codecov bot commented Mar 1, 2024 • edited Loading

Codecov Report

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot commented Mar 4, 2024

github-actions bot commented Mar 4, 2024

Drewniok commented Mar 4, 2024

marcelwa left a comment

Choose a reason for hiding this comment

marcelwa commented Mar 4, 2024

github-actions bot commented Mar 4, 2024

github-actions bot commented Mar 4, 2024

github-actions bot commented Mar 4, 2024

github-actions bot commented Mar 4, 2024

github-actions bot commented Mar 5, 2024

github-actions bot commented Mar 5, 2024

github-actions bot commented Mar 7, 2024

marcelwa left a comment

Choose a reason for hiding this comment

🐛 Fixed bug in three-state simulation and duplicate cds results for `QuickExact`. #388

🐛 Fixed bug in three-state simulation and duplicate cds results for `QuickExact`. #388

Drewniok commented Mar 1, 2024 •

edited

Loading

codecov bot commented Mar 1, 2024 •

edited

Loading