Skip to content
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.

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

foreach_cell iteration should continue until at least one site is found #222

Merged
merged 1 commit into from
Oct 21, 2023

Conversation

pablosanjose
Copy link
Owner

In supercell searches without a specific list of cells, we ensure BoxIterator keeps going until we find at least one site (or we exceed the maximum iterations). However foreach_cell did not have this check, and would stop as soon as a closed shell without hits was formed. This led to lattice slicing (using a region such as a ring around the origing) where we could fail to form a slice. Note also that siteselector has no seed option currently (unlike supercell), so this problem could not be worked around.

Perhaps a future refactor could employ foreach_cell and foreach_site in supercell, to be more DRY.

@pablosanjose pablosanjose changed the title siteselector cell iteration should continue until found foreach_cell iteration should continue until at least one site is found Oct 21, 2023
@codecov-commenter
Copy link

codecov-commenter commented Oct 21, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (db3b60a) 92.58% compared to head (743016a) 92.59%.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #222   +/-   ##
=======================================
  Coverage   92.58%   92.59%           
=======================================
  Files          34       34           
  Lines        5450     5457    +7     
=======================================
+ Hits         5046     5053    +7     
  Misses        404      404           
Files Coverage Δ
src/apply.jl 92.51% <100.00%> (+0.16%) ⬆️
src/selectors.jl 98.80% <100.00%> (+0.04%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@pablosanjose pablosanjose merged commit 6615b9b into master Oct 21, 2023
8 checks passed
@pablosanjose pablosanjose deleted the search_till_first branch October 25, 2024 15:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants