HomSap/Zigzag_1S14 population sizes don't match Schiffels & Durbin (2014)

[grahamgower]

Short description of the problem:
The population sizes in the Zigzag_1S14 demographic model swing between 1431 and 14312. However, the values in Schiffels & Durbin (2014) are five times higher.

How to reproduce the problem:
Compare the zigzag parameters in stdpopsim's catalog with Figure 2 in Schiffels & Durbin (2014). Also see the ms command in the supplementary material, section 7, pg. 25.

This appears to have been deliberate, according to #105, but is not documented, and doesn't seem to have come up during the model QC process (#498).

Suggested fixes:
The original ms command given in the supplement includes a -eN 0 5 command, which has not been reproduced in the stdpopsim model. From the ms manual:
-eN t x Set all subpop’s to size x ∗ N0 and growth rates to zero.

We should at the very least document the difference between the stdpopsim ZigZag model, and that in the original publication.

CC: @Chris1221, @apragsdale.

[EDIT: translation of the ms command, as stolen from the QC implementation]

[apragsdale]

Yep, that looks like a mistake on our part. We should probably fix those values in our implementation to follow the original publication, instead of just documenting it. Maybe the next major release could emit a warning the first time someone uses the zigzag model, that the previous version of stdpopsim had sizes that were 5x smaller than intended?

I'm sorry for missing that in the QC. I'll be very happy the day that I never have to translate an ms command again.

[jeromekelleher]

Happy day indeed! It'll come: popsim-consortium/demes-python#102

(more quickly, if someone would like to pick this PR up...)

[Chris1221]

I agree, that does look like a mistake in our implementation. My apologies for missing that initially, my ms-fu is pretty weak and I completely didn't see the scaling factor. Comparing it to the literature values would have been a good sanity check that I also missed... I agree with @apragsdale's warning suggestion, it's important that people know that it was incorrect!

[Chris1221]

Also, (I think) it looks like this change may need to be propagated to the values in the Zigzag model description in the demes-docs gallery: https://popsim-consortium.github.io/demes-docs/main/gallery.html#zigzag-model

[grahamgower]

Also, (I think) it looks like this change may need to be propagated to the values in the Zigzag model description in the demes-docs gallery: https://popsim-consortium.github.io/demes-docs/main/gallery.html#zigzag-model

Yep, thanks @Chris1221, I've opened an issues in Demes for this too: popsim-consortium/demes-python#179.

[apragsdale]

Also, (I think) it looks like this change may need to be propagated to the values in the Zigzag model description in the demes-docs gallery: https://popsim-consortium.github.io/demes-docs/main/gallery.html#zigzag-model

Yes, for sure. I think I used our implementation from stdpopsim to translate over to demes. Great example of how easily these errors can propagate.. similar to the OOA model issue from earlier this year. We'll need fix it there as well.

[jeromekelleher]

Great to catch this issue, but I don't think it's that serious in the scheme of things since it's a toy model. It's just another example of how easy it is to make mistakes with today's toolchain. So, no worries @Chris1221 and @apragsdale!

In terms of compatibility, I'm not sure what to do. I guess emitting a warning when it's used is probably the best we can do, which we remove after (say) 6 months.