Skip to content

add dog demographic model#905

Closed
agladstein wants to merge 1 commit intopopsim-consortium:mainfrom
agladstein:add-dog-demographic-model-Freedman14
Closed

add dog demographic model#905
agladstein wants to merge 1 commit intopopsim-consortium:mainfrom
agladstein:add-dog-demographic-model-Freedman14

Conversation

@agladstein
Copy link
Contributor

I am making this a draft because I have a few unresolved issues.

  • I cannot find the Boxer Ne
  • When I try to build the docs locally, I do not see the demographic model update to the catalog
  • Also, I think the ancestral populations probably shouldn't be "sampled". I don't know how to do that.

This PR will close #830

@codecov
Copy link

codecov bot commented Apr 10, 2021

Welcome to Codecov 🎉

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

Thanks for integrating Codecov - We've got you covered ☂️

@agladstein agladstein marked this pull request as draft April 12, 2021 15:26
@agladstein
Copy link
Contributor Author

The Boxer Ne is not inferred, so I'm not going to include the Boxer in the model.

Copy link
Member

@apragsdale apragsdale left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @agladstein - looks like it's very close. Just commenting that the mutation rate can now be added to the DemographicModel as that PR was merged, and a question about the mutation rates.

(G-PhoCS) (Gronau et al. 2011) on neutral autosomal loci from
whole genome sequences. See Figure 5A in Freedman et al. 2014.
Note that Freedman et al. 2014 calibrated the parameters with a
mutation rate of 1e-8.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The mutation rate can now be added to the DemographicModel.

m_GLJ_ISW = 0
m_ISW_GLJ = 0.05
m_GLJ_ancDW = 0.02
m_ancDW_GLJ = 0.99
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some of these seem like very high mutation rates.. are they unscaled?

@petrelharp
Copy link
Contributor

If you can finish this up we could get it QC'ed, probably, @agladstein!

@agladstein
Copy link
Contributor Author

Ok, I'll try to do it this weekend.

@agladstein
Copy link
Contributor Author

I started to work on this, but I'm so out of it (computer isn't setup anymore, weird merge conflicts...), I won't be able to finish it this weekend.
Can anyone else pick this up to get it done?

@petrelharp
Copy link
Contributor

I'll ask on slack!

@rwaples
Copy link
Contributor

rwaples commented May 17, 2022

I can give this a try.
At a first glance it looks almost done - I'll see if can tackle the issues @agladstein mentioned at the top.
My initial plan would be paste the changes over to a new fork. I think that will be easier for me rather than trying to pick up this merge. Does that seem like a bad idea?

@agladstein
Copy link
Contributor Author

That seems reasonable to me @rwaples. I got blocked over the weekend because of merge conflicts, so that sounds like a reasonable way around that problem.
I think it is almost done! I just don't have the bandwidth right now.

@rwaples
Copy link
Contributor

rwaples commented May 18, 2022

Sounds good - you did almost all the hard work.

I'd be curious what your thoughts on including the Boxer population.
Based on my reading of the paper, they don't seem to have any samples from the boxer, just the haploid dog genome, which is from a Boxer.
As you noted they don't seem to report any population sizes for the Boxer, but they do give a split time between Basenji and Boxer, as well as an ancestral (Boxer, Basenji) population size.

I see a few options:

  1. Give Boxer an arbitrary size (e.g. 1000), as you have in your draft PR
  2. Give Boxer a size of 1, in effect signaling we don't know
  3. Give Boxer a population size of 0 - either explicitly or implicitly removing it from the model, so that it cannot be sampled from.
  4. Ask the paper authors if they can help
    Any thoughts?

@petrelharp
Copy link
Contributor

Here's some options I can think of:

  1. remove Boxer from the model
  2. set the population size equal to the parent population size (Boxer-Basenji?)
  3. pull an effective population size from some place else (maybe here?)

IMO, any of these would seem reasonable and so would be fine, as long as it's documented what's happened.

@agladstein
Copy link
Contributor Author

I have edits locally that deal with boxer, so apparently me a year ago had decided on something (but merge conflicts making it difficult). I just removed Boxer from the model.
Screen Shot 2022-05-18 at 10 03 46 AM
Screen Shot 2022-05-18 at 10 04 33 AM

@rwaples
Copy link
Contributor

rwaples commented May 18, 2022

Thanks for the input.
Removing the Boxer makes sense to me, as it doesn't seem very appropriate to use this model to simulate Boxer.

@gregorgorjanc
Copy link
Contributor

This PR can be closed due to #1632

@petrelharp petrelharp closed this Jan 6, 2025
petrelharp added a commit that referenced this pull request Jan 6, 2025
* Removing Boxer reduced number of pops from 7 to 6
Co-authored-by: peter <[email protected]>
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

Successfully merging this pull request may close these issues.

Add dog demographic model

5 participants