Improve handling of non-integer sizes in demes models. #930
Conversation
|
@apragsdale -- one issue here is that np.rint is (or claims to be) an "unbiased" round, meaning that round-to-even is the default. So, 100.5 -> 100, yet 101.5 -> 102. That behavior could cause problems. |
molpopgen
force-pushed
the
non_integer_times_in_demes_models
branch
from
78efdc9
to
f80e5c8
Compare
| @@ -517,22 +517,19 @@ def _process_epoch( | |||
| if e.start_time != math.inf: | |||
| assert size_history.deme_exists_at(idmap[deme_id], when + 1) | |||
|
|
|||
| start_size = int(np.rint(e.start_size)) | |||
There was a problem hiding this comment.
Here is a good place to ensure that:
- end_size > start_size (???)
- the duration is > 0 when converted to integer?
There was a problem hiding this comment.
Why do we need to check that end_size > start_size?
But yes, we need to check that duration is 1 generation or greater, I agree.
There was a problem hiding this comment.
My mistake -- I meant start/end time, meaning epoch length > 0. We'll take care of that in another PR so that the git history reflects both this change and the check on epoch lengths.
| @@ -517,22 +517,19 @@ def _process_epoch( | |||
| if e.start_time != math.inf: | |||
| assert size_history.deme_exists_at(idmap[deme_id], when + 1) | |||
|
|
|||
| start_size = int(np.rint(e.start_size)) | |||
There was a problem hiding this comment.
Why do we need to check that end_size > start_size?
But yes, we need to check that duration is 1 generation or greater, I agree.
Let's do that separately. Are you worried about the round to even behavior? |
molpopgen
force-pushed
the
non_integer_times_in_demes_models
branch
2 times, most recently
from
9873217
to
2c5a5ea
Compare
molpopgen
changed the title
|
I’m not specifically concerned by the round the even behavior. I could come up with some specific scenario where it matters, like if all integer times are then scaled by 1/2. In practice, it’s not that likely to be an issue. |
molpopgen
force-pushed
the
non_integer_times_in_demes_models
branch
from
2c5a5ea
to
595b0cf
Compare
See issue #929