🐛 Increased tolerance to avoid undetected degeneracy and added forbidden BDL types in wire detection #685

Drewniok · 2025-03-03T17:17:59Z

Description

This PR increases the tolerance used in groundstate_from_simulation_results to extract the ground states from simulation results. Previously, it failed to detect the degeneracy of the ground state in some cases, leading to "weird" operational domains. This should now be fixed.

Example:
This operational domain should be empty, which is only the case if the degeneracy of the ground state is correctly detected. As said, this should be fixed now.

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.

…degeneracy

…on_detected_degeneracy' into increase_error_margin_to_avoid_non_detected_degeneracy

marcelwa · 2025-03-03T17:55:11Z

Thanks for the fix! 🙏

Slightly off-topic, but thinking about the function, I wonder if we should refactor it into a member of sidb_simulation_result called groundstate for more intuitive usability. What do you think, @Drewniok and @wlambooy?

codecov · 2025-03-03T17:59:21Z

Codecov Report

Attention: Patch coverage is 97.08029% with 4 lines in your changes missing coverage. Please review.

Project coverage is 98.36%. Comparing base (bbb9a09) to head (4ae106b).
Report is 71 commits behind head on main.

Files with missing lines	Patch %	Lines
...on/algorithms/simulation/sidb/detect_bdl_wires.hpp	85.71%	4 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #685      +/-   ##
==========================================
- Coverage   98.36%   98.36%   -0.01%     
==========================================
  Files         254      253       -1     
  Lines       41399    41485      +86     
  Branches     1878     1884       +6     
==========================================
+ Hits        40723    40806      +83     
- Misses        676      679       +3

Files with missing lines	Coverage Δ
...on/algorithms/simulation/sidb/defect_influence.hpp	`96.51% <100.00%> (ø)`
...ion/algorithms/simulation/sidb/is_ground_state.hpp	`100.00% <100.00%> (ø)`
...tion/algorithms/simulation/sidb/is_operational.hpp	`92.02% <100.00%> (-0.03%)`	⬇️
...orithms/simulation/sidb/sidb_simulation_result.hpp	`100.00% <100.00%> (ø)`
...est/algorithms/simulation/sidb/clustercomplete.cpp	`99.68% <100.00%> (ø)`
...lation/sidb/exhaustive_ground_state_simulation.cpp	`100.00% <100.00%> (ø)`
.../algorithms/simulation/sidb/operational_domain.cpp	`100.00% <100.00%> (ø)`
test/algorithms/simulation/sidb/quickexact.cpp	`100.00% <ø> (ø)`
test/algorithms/simulation/sidb/quicksim.cpp	`100.00% <100.00%> (ø)`
...orithms/simulation/sidb/sidb_simulation_result.cpp	`100.00% <100.00%> (ø)`
... and 2 more

... and 1 file 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 abadc97...4ae106b. Read the comment docs.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

github-actions · 2025-03-03T19:18:31Z

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

Drewniok · 2025-03-03T20:07:03Z

Thanks for the fix! 🙏

Slightly off-topic, but thinking about the function, I wonder if we should refactor it into a member of sidb_simulation_result called groundstate for more intuitive usability. What do you think, @Drewniok and @wlambooy?

That's a great idea! I like it!

…put wire and vice versa.

Signed-off-by: GitHub Actions <actions@github.com>

github-actions · 2025-03-04T09:23:57Z

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

…degeneracy

github-actions · 2025-03-04T11:11:49Z

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

wlambooy · 2025-03-04T19:33:40Z

Thanks for the fix! 🙏

Slightly off-topic, but thinking about the function, I wonder if we should refactor it into a member of sidb_simulation_result called groundstate for more intuitive usability. What do you think, @Drewniok and @wlambooy?

I think that is an excellent idea! Since there is always the possibility of getting degenerate groundstates, we should respect that in the function name, i.e.: groundstates.

marcelwa · 2025-03-04T21:23:54Z

Thanks for the fix! 🙏

Slightly off-topic, but thinking about the function, I wonder if we should refactor it into a member of sidb_simulation_result called groundstate for more intuitive usability. What do you think, @Drewniok and @wlambooy?

I think that is an excellent idea! Since there is always the possibility of getting degenerate groundstates, we should respect that in the function name, i.e.: groundstates.

Good call! Would it make sense then to parameterize the function to allow degeneracy or not? Or have two separate functions for that? I'm also okay with having a single one simply called groundstates. I'm merely asking for your preference since you're going to use the API the most.

Drewniok · 2025-03-05T05:48:11Z

Thanks for the fix! 🙏
Slightly off-topic, but thinking about the function, I wonder if we should refactor it into a member of sidb_simulation_result called groundstate for more intuitive usability. What do you think, @Drewniok and @wlambooy?

I think that is an excellent idea! Since there is always the possibility of getting degenerate groundstates, we should respect that in the function name, i.e.: groundstates.

Good call! Would it make sense then to parameterize the function to allow degeneracy or not? Or have two separate functions for that? I'm also okay with having a single one simply called groundstates. I'm merely asking for your preference since you're going to use the API the most.

Personally, I do not see a reason for parameterizing it.

…degeneracy

Signed-off-by: GitHub Actions <actions@github.com>

github-actions

clang-tidy made some suggestions

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

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

test/algorithms/simulation/sidb/quickexact.cpp

test/algorithms/simulation/sidb/quicksim.cpp

Signed-off-by: GitHub Actions <actions@github.com>

bindings/mnt/pyfiction/include/pyfiction/algorithms/simulation/sidb/sidb_simulation_result.hpp

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

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

test/algorithms/simulation/sidb/quickexact.cpp

Signed-off-by: GitHub Actions <actions@github.com>

…on_detected_degeneracy' into increase_error_margin_to_avoid_non_detected_degeneracy

…degeneracy

Signed-off-by: GitHub Actions <actions@github.com>

…on_detected_degeneracy' into increase_error_margin_to_avoid_non_detected_degeneracy

…degeneracy

…on_detected_degeneracy' into increase_error_margin_to_avoid_non_detected_degeneracy

Signed-off-by: GitHub Actions <actions@github.com>

…degeneracy

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

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

…on_detected_degeneracy' into increase_error_margin_to_avoid_non_detected_degeneracy

Signed-off-by: GitHub Actions <actions@github.com>

…degeneracy

marcelwa

Many thanks for the fix and the quick iterations!

🐛 increase tolerance to avoid non-detected degeneracy.

b8b6784

Drewniok added the bug Something isn't working label Mar 3, 2025

Drewniok self-assigned this Mar 3, 2025

Drewniok added 2 commits March 3, 2025 18:20

🎨 small fix.

a05fb40

🎨 small fix.

88a8dd4

Drewniok changed the title ~~🐛 Increased tolerance to avoid undetected degeneracy.~~ 🐛 Increased tolerance to avoid undetected degeneracy Mar 3, 2025

Drewniok added 3 commits March 3, 2025 18:24

Merge branch 'main' into increase_error_margin_to_avoid_non_detected_…

4ad44e1

…degeneracy

Merge remote-tracking branch 'origin/increase_error_margin_to_avoid_n…

5f2b139

…on_detected_degeneracy' into increase_error_margin_to_avoid_non_detected_degeneracy

🎨 small fix.

2a019a3

Drewniok requested a review from marcelwa March 3, 2025 17:34

Drewniok and others added 3 commits March 4, 2025 07:41

✅ add test.

7abc763

🐛 important bug fix! input BDL pairs should be filtered out in an out…

04a5a12

…put wire and vice versa.

📝 Update pyfiction docstrings

296dea4

Signed-off-by: GitHub Actions <actions@github.com>

Drewniok changed the title ~~🐛 Increased tolerance to avoid undetected degeneracy~~ 🐛 Increased tolerance to avoid undetected degeneracy and add forbidden BDL types in wire detection Mar 4, 2025

Drewniok changed the title ~~🐛 Increased tolerance to avoid undetected degeneracy and add forbidden BDL types in wire detection~~ 🐛 Increased tolerance to avoid undetected degeneracy and added forbidden BDL types in wire detection Mar 4, 2025

Merge branch 'main' into increase_error_margin_to_avoid_non_detected_…

db36229

…degeneracy

Drewniok and others added 4 commits March 5, 2025 21:52

🎨 put ground state function to simulation result.

beca611

Merge branch 'main' into increase_error_margin_to_avoid_non_detected_…

51bce63

…degeneracy

📝 Update pyfiction docstrings

9e04638

Signed-off-by: GitHub Actions <actions@github.com>

🎨 rename and small fixes.

5221dc2

github-actions bot reviewed Mar 5, 2025

View reviewed changes

📝 Update pyfiction docstrings

243445f

Signed-off-by: GitHub Actions <actions@github.com>

marcelwa requested changes Mar 6, 2025

View reviewed changes

Drewniok and others added 16 commits March 8, 2025 12:27

🎨 integrate Marcel's feedback.

2a8e0dc

📝 Update pyfiction docstrings

f27a522

Signed-off-by: GitHub Actions <actions@github.com>

🎨 small renaming.

fb995bd

Merge remote-tracking branch 'origin/increase_error_margin_to_avoid_n…

9df5271

…on_detected_degeneracy' into increase_error_margin_to_avoid_non_detected_degeneracy

Merge branch 'main' into increase_error_margin_to_avoid_non_detected_…

f5e7a8e

…degeneracy

📝 Update pyfiction docstrings

05e4557

Signed-off-by: GitHub Actions <actions@github.com>

Merge remote-tracking branch 'origin/increase_error_margin_to_avoid_n…

76a1c0d

…on_detected_degeneracy' into increase_error_margin_to_avoid_non_detected_degeneracy

Merge branch 'main' into increase_error_margin_to_avoid_non_detected_…

5f441cc

…degeneracy

Merge branch 'main' into increase_error_margin_to_avoid_non_detected_…

8315b2b

…degeneracy

Merge remote-tracking branch 'origin/increase_error_margin_to_avoid_n…

e19f321

…on_detected_degeneracy' into increase_error_margin_to_avoid_non_detected_degeneracy

🎨 update experiment.

88ca4d1

🎨 rename.

b4b1559

🎨 small restructering.

460da45

📝 Update pyfiction docstrings

7beac63

Signed-off-by: GitHub Actions <actions@github.com>

🎨 small restructering.

931dfc7

🎨 small fix.

8691e3c

Drewniok requested a review from marcelwa March 13, 2025 12:12

Merge branch 'main' into increase_error_margin_to_avoid_non_detected_…

859e2a4

…degeneracy

marcelwa requested changes Mar 14, 2025

View reviewed changes

Drewniok and others added 4 commits March 16, 2025 19:47

🎨 implement Marcel's feedback.

7213b7b

Merge remote-tracking branch 'origin/increase_error_margin_to_avoid_n…

74a92e4

…on_detected_degeneracy' into increase_error_margin_to_avoid_non_detected_degeneracy

📝 Update pyfiction docstrings

1c4ec37

Signed-off-by: GitHub Actions <actions@github.com>

Merge branch 'main' into increase_error_margin_to_avoid_non_detected_…

4ae106b

…degeneracy

Drewniok requested a review from marcelwa March 16, 2025 18:57

marcelwa approved these changes Mar 17, 2025

View reviewed changes

Drewniok merged commit 4c27663 into cda-tum:main Mar 17, 2025
49 checks passed

🐛 Increased tolerance to avoid undetected degeneracy and added forbidden BDL types in wire detection #685

🐛 Increased tolerance to avoid undetected degeneracy and added forbidden BDL types in wire detection #685

Uh oh!

Conversation

Drewniok commented Mar 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist:

Uh oh!

marcelwa commented Mar 3, 2025

Uh oh!

codecov bot commented Mar 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

github-actions bot commented Mar 3, 2025

Uh oh!

Drewniok commented Mar 3, 2025

Uh oh!

github-actions bot commented Mar 4, 2025

Uh oh!

github-actions bot commented Mar 4, 2025

Uh oh!

wlambooy commented Mar 4, 2025

Uh oh!

marcelwa commented Mar 4, 2025

Uh oh!

Drewniok commented Mar 5, 2025

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

marcelwa left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Drewniok commented Mar 3, 2025 •

edited

Loading

codecov bot commented Mar 3, 2025 •

edited

Loading