popsim-consortium / analysis2

Analysis for the second consortium paper.
8 stars 14 forks source link

A potential bug in stairway plot N(t) analysis? #71

Closed lntran26 closed 2 years ago

lntran26 commented 2 years ago

When running the N(t) analysis on two demographic models "Constant" and Out-Of-Africa (YRI), @emanuelmfonseca and I noticed that the stairway plot results for these models are virtually identical (while msmc and GONE results do differ). @lauterbur noticed this on one of the calls where we showed the initial 1 rep result. We confirm that this happens consistently across different runs and on multiple replicates.

I also found that when running multiple replicates, the number of lines plotted for stairway plots using the current pipeline often don't match with the number of replicates specified in the config file, while msmc and GONE always do. Since tiny_config.yaml currently only have 1 rep and 1 demography these issues might be missed when debugging.

constant_all_estimated_Ne OOF_all_estimated_Ne

andrewkern commented 2 years ago

ah okay! this should be an easy enough bug fix. have you spotted the source of the issue yet?

andrewkern commented 2 years ago

I found the bug. The list of files stairwayplot intermediate files being handed to the all_plot rule was incorrect, and had a list of all demographic models used. I will submit a simple PR to fix this.

lntran26 commented 2 years ago

Thank you @andrewkern ! @emanuelmfonseca and I have attempted to find/fix the bug ourselves but have been unsuccessful. Glad you can find it rather quickly!

andrewkern commented 2 years ago

@lntran26 I just authored a PR to fix this.