Msprime: rename "kept"-related options in sim_mutations

[petrelharp]

As discussed in #1179, the current API around mutation on top of existing mutations is a bit confusing. One issue is that if you mutate a tree sequence that already has mutations, those mutations silently disappear unless you say keep=True, which is easy to forget. Also: it does not make sense for many mutation models to put new mutations ancestral to existing ones, so if that could happen then we throw an error, unless the option kept_mutations_before_end_time is True. But, the name of this option is confusing and unwieldy.

Proposal:

• change mutate so that if it is provided with tables that have any mutations in, it throws an error, unless add=True
• rename the kept_mutations_before_end_time option to add_ancestral.

Thoughts, @mufernando? @GertjanBisschop?

[mufernando]

I really like this idea. What are we going to do about backwards compatibility?

[petrelharp]

The old behavior has not been actually released, so we're still free to change it.

[petrelharp]

Sorry, that's wrong, keep was released. So we'll keep keep but throw a deprecation warning if it's used.

[jeromekelleher]

@petrelharp, @mufernando, what's the proposal here? We now have a blank slate with sim_mutations and don't have to consider compatibility. What should we do?

[petrelharp]

Oh, excellent. Let's see. I think the initial proposal above is good. Restating:

def sim_mutations(..., keep=True, add_ancestral=False):
    """

    If the tree sequence already has mutations, these are by default retained, but can be discarded
    by passing ``keep=False``. However, adding new mutations to a tree sequence with existing mutations must be
    done with caution, since it can lead to incorrect or nonsensical results if mutation probabilities differ
    by ancestral state. (As an extreme example,
    suppose that X->Y and X->Z are allowable transitions, but Y->Z is not. If a branch already has an
    X->Y mutation on it, then calling `sim_mutations(..., keep=True)` might insert an X->Z mutation
    above the existing mutation, thus implying the impossible chain X->Y->Z.)  For this reason, if
    this method attempts to add a new mutation ancestral to any existing mutation, an error will occur,
    unless ``add_ancestral=True``. The ``add_ancestral`` parameter has no effect if ``keep=False``. 
    In summary, to add more mutations to a tree sequence with existing
    mutations, you need to either ensure that no new mutations are ancestral to existing ones (e.g.,
    using the ``end_time`` parameter), or set ``add_ancestral=True``.

    :param bool keep: Whether to keep existing mutations. (default: True)
    :param bool add_ancestral: Whether to allow the addition of new mutations ancestral to existing ones. (default: False)
    """

[jeromekelleher]

SGTM!

[jeromekelleher]

Best get the basic change into the alpha4 milestone, so we can start updating pyslim and stdpopsim.

[jeromekelleher]

I'll get this one in.