Msprime: Add new PopulationMerger/PopulationSplit event?

[jeromekelleher]

Given the confusion created by MassMigration events and that fact that we have repeatedly forgotten to set migration rates to zero when populations merge (going backwards in time), I think we should add a new event which does this. Specifically, move all lineages out of the source population and into the dest, and set all migration rates into source to zero. This is the same semantics as ms's -ej event.

It's simple to implement (if a bit tedious) but would be well worth the effort.

The real question is, what do we call it? I would find it less confusing and easier to explain if we defined it in terms of the coalescent perspective and so it's seen as a PopulationMerger. But I can see this being confusing, and maybe it's better to call it a PopulationSplit following ms. Suggestions very welcome!

Pinging @petrelharp, @grahamgower, @apragsdale, @ndukler and anyone else who has an opinion here.

Refs:

• #981.
• https://github.com/popsim-consortium/stdpopsim/issues/516
• https://github.com/popsim-consortium/stdpopsim/issues/496

[molpopgen]

I think that the confusion would be solved by a few examples up front in the manual. Any new types would probably simply wrap the existing functionality anyways, so I'd think it is better to maintain backwards compatibility and not have several types doing the same thing.

[jeromekelleher]

We'll keep the existing types, so there's no backwards compatibility issues. What I'm suggesting is really an event that is equivalent to set of other events. So, this would be equivalent if we had 4 populations:

events = [
    msprime.MassMigration(time=1, source=1, dest=0, proportion=1),  # Move everyone in 1 to 0
    mpsrime.MigrationRateChange(time=1, source=1, dest=0, rate=0), # Switch off migration into 0
    mpsrime.MigrationRateChange(time=1, source=2, dest=0, rate=0),
    mpsrime.MigrationRateChange(time=1, source=3, dest=0, rate=0),
]

and

events = [
    msprime.PopulationMerger(time=1, source=1, dest=0) # Move everyone in 1 into 0 and 1 is now out of the model
]

This seems less error prone, no?

[petrelharp]

I vote for describing it in real terms, so: PopulationSplit. Or I guess you could go for a time-direction-agnostic description, like PopulationBranchPoint.

[jeromekelleher]

Sounds good @petrelharp - what are the parameter names you'd use?

[apragsdale]

I agree with @petrelharp that using real/forward-thinking terms is easier to understand. But that makes source and dest a bit awkward (if I think of a population split event with source 1 and destination 0, I'd think of splitting the population in deme 1 into demes 0 and 1, instead of the opposite). So maybe PopulationMerger makes sense if we want to be consistent with the source/dest language regarding the movement of lineages backward in time?

Edit: Something like "PopulationCoalescence" that makes it clear that the thinking is backward in time would work? Though I don't really like the "coalescence" bit of the name

[petrelharp]

I really don't think we should call it PopulationMerger, since there will be people who use it, thinking that it does a population merger, when it does the opposite.

Good point: but, this is a new method, so we don't have to use "source" and "dest" at all. Perhaps just pop1 and pop2, then say in the docs exactly what happens? Or ancestral_pop and derived_pop?

[apragsdale]

Agree that PopulationMerger would be confusing. Maybe PopulationSplit(ancestral_pop=A, derived_pops=[A, B, C, ...]) allows for the split into two or more populations?

I always have to stop and think about how to use source/dest and double check I'm not sending migrations in the wrong direction.. it is confusing.

[molpopgen]

If we add new event classes, then perhaps they should be mutually incompatible, meaning that a model is invalid if its demographic_events contains both MassMigration and the new types? It seems to only add to confusion if we allow the older type causing confusion to be mixed in with the new types if the default handling of migration rates differs b/w the two.

[grahamgower]

In general, I think this is a great idea. +1 for PopulationSplit. And to throw another parameterisation option into the mix, how about PopulationSplit(populations=[A, B, ...]), where A splits into A and B (and ...) forwards in time?

[grahamgower]

If we add new event classes, then perhaps they should be mutually incompatible, meaning that a model is invalid if its demographic_events contains both MassMigration and the new types?

What about MassMigration with proportion<1?

[molpopgen]

What about MassMigration with proportion<1?

Then you still risk mixing types with different default side-effects. Documenting that will be tricky.

[apragsdale]

In general, I think this is a great idea. +1 for PopulationSplit. And to throw another parameterisation option into the mix, how about PopulationSplit(populations=[A, B, ...]), where A splits into A and B (and ...) forwards in time?

I think it would be good to be very explicit about which one is the parental population, since PopulationSplit([A, B, ...]) and PopulationSplit([B, A, ...]) would have different behavior and could lead to unintended model misspecification. If the goal is to make model mistakes less likely, having the extra keyword is safer I think.

[grahamgower]

If the goal is to make model mistakes less likely, having the extra keyword is safer I think.

Yeah, you're probably right. PopulationSplit(ancestral_pop=A, derived_pops=[A, B, C, ...]) also has the advantage that the ancestral_pop needn't be in the derived_pops.

[jeromekelleher]

Another thought for the mix: what if we added another Admixture event (with appropriately named parameters) and then deprecate (but keep indefinitely) MassMigration? Then the only place we have the source and dest names is in the MigrationRateChange, and these have not been released yet. In the current version, the parameter to MigrationRateChange is matrix_index, which is perhaps better.

Basically, I now think Hudson was right all along and my attempt to improve on the interface to ms was a big mistake.