Add MutationTable.time column

[jeromekelleher]

Currently we have no way of recording the precise age of a mutation. All that we know is that it occured somewhere on a branch. Most of the time this isn't an issue, but there are situtations in which we need to record this precise time (see, tskit-dev/msprime#946, e.g.).

We can add a MutationTable.time column easily enough. If the mutation time is not specified (the default) then we set it to the time of the mutation's node. If multiple mutations are added on the same branch without times being specified, then they will have the same time (oh well, no big deal I guess).

In terms of the file format, we make this optional in the same way that Edge metadata is optional (it would be simplest to call set_columns to implement this though). It might be best to do #506 first, therefore.

As this is backward compatible, we don't need to increment the file version number.

We will break some C code since mutation_table_add_row et al will need a new parameter, which most naturally comes between the current parent and derived_state. I guess providing TSK_NULL indicates you want the default for this. Since we're already breaking some C code, this seems fine.

@benjeffery, any thoughts here?

[petrelharp]

Supporting information that this is a good idea: SLiM already records the time, but in metadata.

[molpopgen]

fwdpy11 does the same thing. If you keep tables in the native (fwdpp) format, the mutation's origin time is with respect to time moving forwards. If you dump to the tskit format, then origin times are changed to be pastwards from the last time point simulated.

[jeromekelleher]

I'm reassured we all think this is a good idea!

[benjeffery]

Do we know why SLiM includes this in metadata? It seems to be a core enough property of a mutation to warrant inclusion as a column.
I agree on getting #506 done first and making this backwards compatible. This will be the first "non-trivial" default value so care will need to be taken in documenting the behaviour.

[jeromekelleher]

Do we know why SLiM includes this in metadata? It seems to be a core enough property of a mutation to warrant inclusion as a column.

It was just because it wasn't implemented in tskit and we decided there were other things to be getting on with, I'd imagine.

[petrelharp]

A though: once this is implemented, we should have sort_tables sort mutations by time, which satisfies the current requirement that parents come before children, but would make the ordering produced by sort_tables less ambiguous (not totally, if in discrete time, though). Perhaps we should make "sorted by time" a requirement of the mutation table, also.

[benjeffery]

I've started working on this. Above it says that if TSK_NULL is passed as the time to tsk_mutation_table_add_row then it should default to the node time. However I don't see how tsk_mutation_table_add_row has access to the node time as it only has the node id.

[benjeffery]

Just had a discussion with @jeromekelleher and @awohns on this issue.

Any way of loading an old TS that doesn't have the mutation time column seems complex and a kludge. The mutation loading code would need to have the node table and do lots of look ups. It is also not clear how tsk_mutation_table_add_row could provide a default.

It seems a cleaner path to make mutation time a mandatory column (bumping the major file version). Then tskit upgrade would add mutation times by spreading them equally across each edge in a one-off procedure. Mutation time would be mandatory in add_row. @petrelharp thoughts?

[petrelharp]

I agree - making mutation time mandatory sounds like the right way to go. Hm: it seems like we'll really want to get msprime and SLiM producing mutation times right away, to avoid a period of awkwardness? This will be easy to add in SLiM, but would require a release to get adoption. I guess what I'm saying is that a lot of people might be encountering this.

About spreading the times out over the branch - also, agree! At first I was nervous about adding extra fake information, but we're not using mutation times currently, so I think it's good.

Hm - it occurs to me that some methods - eg tsinfer? - don't have any natural way to assign times along a branch, so it could be useful to allow clients to ignore times in tables (eg leave them all NULL), and provide a method to assign times uniformly along a branch, much like compute_mutation_parents and deduplicate_sites? Since we need to write this method anyhow? (to be clear, I think the time column should make sense for a valid tree sequence, I'm talking about 'just a collection of tables')

This will be great to get in there for the future "time slice" statistics!