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

SimulationStatus semantics #934

Closed
molpopgen opened this issue May 17, 2022 · 4 comments
Closed

SimulationStatus semantics #934

molpopgen opened this issue May 17, 2022 · 4 comments
Milestone

Comments

@molpopgen
Copy link
Owner

The docstring for this type doesn't do a great job of explaining what the expectations are.

Further, existing tests are invariant to how this type is used in for loop checking existing fixations in the implementation of GlobalFixation. So either the tests aren't covering that block or something is up.

@molpopgen molpopgen added this to the 0.18.0 milestone May 17, 2022
@molpopgen molpopgen removed this from the 0.18.0 milestone May 26, 2022
@molpopgen
Copy link
Owner Author

Removing this from the 0.18.0 milestone -- I need to think of what needs testing and how to test it.

@molpopgen molpopgen added this to the 0.21.0 milestone Aug 13, 2023
@molpopgen
Copy link
Owner Author

Further, existing tests are invariant to how this type is used in for loop checking existing fixations in the implementation of GlobalFixation. So either the tests aren't covering that block or something is up.

Here, there is some logic that checks if a mutation's key has changed and the mutation is now in the fixations vector. It is either not possible to hit this block or our tests don't cover this case.

@molpopgen
Copy link
Owner Author

molpopgen commented Aug 14, 2023

Here, there is some logic that checks if a mutation's key has changed and the mutation is now in the fixations vector. It is either not possible to hit this block or our tests don't cover this case.

Some exploration leading to #1160 indicates that this code segment is not getting hit at all. I should probably determine coverage for this module.

Edit: the logic is incorrect in the code not getting executed: the mutation index will always/most often yield the same key value because the mutation hasn't had a chance to be recycled yet.

@molpopgen
Copy link
Owner Author

One potential way forward:

  1. Change the status class to an enum with variants Restart, Continue, Complete.
  2. Handle the "simulate to end of model" concept after the while loop handling the conditional part. With demes models, it is now trivial for us to pick up where we left off, etc..

molpopgen added a commit that referenced this issue Aug 15, 2023
* SimulationStatus is now an enum.
* Simplify implementations of some of the condition monitors.
* Update vignette

Closes #934
molpopgen added a commit that referenced this issue Aug 16, 2023
* SimulationStatus is now an enum.
* Simplify implementations of some of the condition monitors.
* Update vignette

Closes #934
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

No branches or pull requests

1 participant