add mus mus domesticus demographic model #1485

peterdfields · 2023-05-20T21:36:21Z

Hi @igronau It's taken me a bit of time to get back to this but you mentioned I should ping you when I submit things for the demographic models and DFE for mouse. Here is a draft demographic model for Mus musculus domesticus from Fujiwara et al. 2022. I have models for the other two subspecies of mouse as well from the same publication, I figured I'd just start with this one to make sure the format was okay. I haven't heard back anything about the genetic map, should I try pinging anyone about that? Anyway, if this looks okay I can add the two other models and then start on the DFE.

igronau

Sorry it took me so much time to get to this. I reviewed the Domesticus demographic model and left some comments. Only one has to do with actual content. The other two are more about format and documentation. Let me know if you have any questions.

igronau · 2023-05-30T16:23:14Z

stdpopsim/catalog/MusMus/demographic_models.py

+
+    return stdpopsim.DemographicModel(
+        id="M_musculus_domesticus_Europe",
+        description="M. musculus domesticus piecewise constant size",


We have certain conventions for the demographic model ids. You can find them described here. According to these conventions, I would suggest something like DomesticusEurope_1F22

Done. I also added similar names for the other demographic models.

Looks good.

igronau · 2023-05-30T16:25:34Z

stdpopsim/catalog/MusMus/demographic_models.py

+            This model comes from MSMC using four randomly sampled
+            individuals (DEU01,DEU03,DEU04,DEU06) from a German population.
+            The model is estimated with 57 time periods.
+        """,


I would also explicitly mention Fig 3 of Fujiwara et al 2022 and say that the population sizes and time changes were supplied by the authors (maybe even mention a specific author?).

I have added these in.

igronau · 2023-05-30T16:45:16Z

stdpopsim/catalog/MusMus/demographic_models.py

+            This model comes from MSMC using four randomly sampled
+            individuals (KOR01,KOR02,KOR03,KOR05) from a Korean population.
+            The model is estimated with 57 time periods.
+        """,


See comments above about the model id and long description

igronau · 2023-05-30T16:52:47Z

stdpopsim/catalog/MusMus/demographic_models.py

+            2040,
+            3844,
+            90428,
+            145603,


check that there is no offset here. From examination of fig 3, it looks like between 300-400 generations ago the Ne was ~150K. The way the arrays are set up here, I think that Ne=145,603 is associated with the time range 420-570.

When you say offset here do you mean something being added at the stage of plotting?

There appears to be an inconsistency between the plot in fig 3 of Fujiwara et al 2022 and your implementation. I don't know if the problem is in fig 3 or your implementation. If I understand the implementation correctly, then each Ne in your table is being associated with the wrong time range (shifted one range back in time). The 5th Ne (145603) is associated in your implementation with the 5th time interval (420 - 570 generations ago). However, in Fig. 3, it appears to be associated the time range somewhere between 300 to 400 generations ago, which fits your 4th time interval. If you remove the first element in the sizes array, this should fix it IMO.

petrelharp · 2023-05-30T17:26:01Z

Thanks a lot! Also note from the automatic checks that we're missing a table of parameters:

Skipping model parameters for M_musculus_domesticus_Europe due to missing table

codecov · 2023-06-03T19:39:50Z

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (b185585) 99.85% compared to head (51386ba) 99.85%.

❗ Current head 51386ba differs from pull request most recent head 6720d15. Consider uploading reports for the commit 6720d15 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1485      +/-   ##
==========================================
- Coverage   99.85%   99.85%   -0.01%     
==========================================
  Files         125      122       -3     
  Lines        4217     4210       -7     
  Branches      588      591       +3     
==========================================
- Hits         4211     4204       -7     
  Misses          3        3              
  Partials        3        3

Files	Coverage Δ
stdpopsim/catalog/MusMus/__init__.py	`100.00% <100.00%> (ø)`
stdpopsim/catalog/MusMus/demographic_models.py	`100.00% <100.00%> (ø)`

... and 5 files with indirect coverage changes

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

peterdfields · 2023-06-03T23:06:55Z

Thanks a lot! Also note from the automatic checks that we're missing a table of parameters:

Skipping model parameters for M_musculus_domesticus_Europe due to missing table

I have added these parameter tables in and the automatics checks seem to be passing now.

igronau · 2023-06-13T16:32:53Z

stdpopsim/catalog/MusMus/demographic_models.py

+
+    return stdpopsim.DemographicModel(
+        id="M_musculus_domesticus_Europe",
+        description="M. musculus domesticus piecewise constant size",


Looks good.

igronau · 2023-06-13T16:34:34Z

docs/parameter_tables/MusMus/DomesticusEurope_1F22.csv

+Time (yrs.),291,Begining of 54th time interval
+Time (yrs.),180,Begining of 55th time interval
+Time (yrs.),83,Begining of 56th time interval
+Time (yrs.),0,Begining of 57th time interval


Parameter table looks okay. I think that the standard is to have one more entry in the list of Nes than in the list of times. This should correspond to the Ne before the first time interval (the ancestral Ne). Other Nes should map to time intervals. This also relates to my comment on the "shift" in the demographic model

igronau

Looks good. See my comment about the number of Ne entries in the Domesticus model

peterdfields · 2023-06-17T05:41:55Z

Looks good. See my comment about the number of Ne entries in the Domesticus model

@igronau I removed the first time interval from the demographic history file and tables, please let me know if any additional changes are needed.

igronau · 2023-07-06T20:18:01Z

@peterdfields , Sorry for the delayed response. I just came back from a long vacation. I think that your changes didn't fix the problem. You just removed he last entry in the times and sizes arrays, but this shouldn't really change how the more recent time intervals are implemented in the demographic model. What I think needs to be done is removing the first entry only in the sizes array (and keeping the times array as is). However, seeing as this is a major source of confusion, then maybe we should try to set a time to discuss this (via Zoom or some other interactive medium)? It might be the simplest way to make sure that we get it right. What do you say?

peterdfields · 2023-07-06T23:02:25Z

@igronau No worries. I'm happy to discuss over zoom. What sort of windows of time work best for you?

igronau · 2023-07-06T23:20:36Z

I'm now in UCLA (Pacific time) and quite flexible in my schedule until next Wednesday. What time works best for you?

peterdfields · 2023-07-07T00:36:05Z

I messaged you over slack. Let me know if you would prefer to continue here.

petrelharp · 2023-10-10T16:54:50Z

ping @igronau @peterdfields - what's going on with this?

peterdfields · 2023-10-10T17:53:55Z

@petrelharp @igronau I made some small changes to the configuration and tables files but it's not obvious to me how these changes caused the (runner?) errors. I just saw the update on Slack about getting the demography for mouse figured out and I remembered I needed to implement @igronau suggestions.

petrelharp · 2023-10-10T19:24:10Z

Those errors are not your fault - looks like random CI failure. I'd say (a) implement the changes you need to make (and push); (b) run tests locally to identify errors; then (c) we'll get CI sorted if there's still an issue.

peterdfields · 2023-10-11T02:00:31Z

@petrelharp the updates pushed yesterday seem to be working fine on the functional side. @igronau do the implemented changes fix the problems you identified before?

igronau · 2023-10-11T11:53:55Z

I verified the model for Domesticus and it seems fine. I created a figure below to validate that the pop size between times[i] and times[i+1] is indeed sizes[i]. The tables appear to require some tweaking. For example, according to the DomesticusEurope_1F22.csv, pop sizes are defined for 55 intervals and not 56, and sizes[1] (2040) is associated with the 55th interval and not the 56th interval. This can be fixed by adding another 133912 value to the table. There are 3 intervals associated with this Ne in the model. This is in addition to the ancestral Ne (also 133912), which is associated with the time range before year 1915544.

If my associations are correct,

igronau · 2023-10-11T16:03:26Z

I now also validated the demographic model for Mus Mus (red line in fig 3). In particular, I checked the first peak in Ne, which according to the plot occurred around 900-750 generations ago. The first local max in sizes is sizes[11] (68111), and times[11]=740 and times[12]=891, which is consistent with the figure. Also here, the parameter table in MusculusKorea_1F22.csv requires tweaking. Again, size[1] (179912) is associated with the 55th time interval and not the 56th interval. This can be fixed by adding another 152757 value to the table. There are 3 intervals associated with this Ne in the model. This is in addition to the ancestral Ne (also 152757), which is associated with the time range before year 807711.

igronau · 2023-10-11T20:29:58Z

I now also validated the demographic model for Mus Cas (green line in fig 3). I checked the first four intervals and they seem consistent with the figure (although the Ne of 938111 between 1886 and 3011 generations ago is not clear in the figure because it is out of bounds). Also here, the parameter table in CastaneusIndia_1F22.csv requires tweaking. Again, size[1] (64853) is associated with the 55th time interval and not the 56th interval. This can be fixed by adding another 344802 value to the table.

peterdfields · 2023-10-16T03:09:06Z

@igronau I've updated the table files with (I hope correct!) the additional values/intervals you described. Please let me know if I should add any other modifications.

igronau · 2023-10-16T15:17:33Z

Looks good. I think that this PR can finally be merged, and then I can formally QC it

igronau · 2023-10-16T15:17:55Z

@petrelharp - can you merge this?

peterdfields · 2023-10-16T15:34:19Z

Looks good. I think that this PR can finally be merged, and then I can formally QC it

yay! On to the DFE...I think this one will probably be the most tricky.

petrelharp · 2023-10-17T06:08:33Z

I've run the tests locally, and they've passed. We'll deal with the CI failures elsewhere (e.g., maybe bumping the cache number like Ive just done in #1526 will do it). So, I'll merge this. Can one of you open the QC issue, please?

peter-d-fields added 4 commits May 20, 2023 16:19

add mus mus dom model

f90f788

small mod to syntax

a38619f

add mus musculus musculus demographic model

20d0430

fix odd vscode related file naming

6ab7bc7

igronau reviewed May 30, 2023

View reviewed changes

peter-d-fields added 2 commits June 3, 2023 02:55

fix first name, add parameter table

5ccf682

add second parameter table

b496de4

peter-d-fields added 2 commits June 3, 2023 17:41

add castaneous demographic model and parameter table

6f9822d

added information about source of model data

14d28c9

igronau reviewed Jun 13, 2023

View reviewed changes

igronau approved these changes Jun 13, 2023

View reviewed changes

fix time intervals in history and parameter table files

abc45cb

peterdfields and others added 2 commits October 8, 2023 22:18

Merge branch 'popsim-consortium:main' into mus_mus_dem_history

51386ba

fix time, size intervals based on Ilan's suggestions

0cf42b0

peterdfields and others added 2 commits October 15, 2023 22:37

Merge branch 'popsim-consortium:main' into mus_mus_dem_history

408af33

add table values to match published demographic models

6720d15

petrelharp merged commit 496a879 into popsim-consortium:main Oct 17, 2023
3 of 9 checks passed

igronau mentioned this pull request Oct 17, 2023

QC for DomesticusEurope_1F22, MusculusKorea_1F22, and CastaneusIndia_1F22 (MusMus) #1528

Closed

peterdfields deleted the mus_mus_dem_history branch October 18, 2023 15:39

+,
+,
+,
+,

add mus mus domesticus demographic model #1485

add mus mus domesticus demographic model #1485

Conversation

peterdfields commented May 20, 2023

igronau left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

petrelharp commented May 30, 2023

codecov bot commented Jun 3, 2023 • edited Loading

Codecov Report

peterdfields commented Jun 3, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

igronau left a comment

Choose a reason for hiding this comment

peterdfields commented Jun 17, 2023

igronau commented Jul 6, 2023

peterdfields commented Jul 6, 2023

igronau commented Jul 6, 2023

peterdfields commented Jul 7, 2023

petrelharp commented Oct 10, 2023

peterdfields commented Oct 10, 2023

petrelharp commented Oct 10, 2023

peterdfields commented Oct 11, 2023

igronau commented Oct 11, 2023

igronau commented Oct 11, 2023

igronau commented Oct 11, 2023

peterdfields commented Oct 16, 2023

igronau commented Oct 16, 2023

igronau commented Oct 16, 2023

peterdfields commented Oct 16, 2023

petrelharp commented Oct 17, 2023

codecov bot commented Jun 3, 2023 •

edited

Loading