Possible (minor) bug in PC ancestors?

[hyanwong]

I see that when we add a PC ancestor here:

tsinfer/tsinfer/algorithm.py

Line 416 in 64ee760

pc_node = self.add_node(-1, constants.NODE_IS_PC_ANCESTOR)

And presumably in an equivalent place in the C code, we override the default "flags=tskit.NODE_IS_SAMPLE" value. This presumably means that when we create PC ancestors in the ancestors_tree_sequence, they are not marked as samples. I think there was a good reason to mark all the nodes in the ancestors_tree_sequence as samples, e.g. so they don't get simplified away. If so, then the PC nodes are escaping this, and I guess that might be responsible for some weird behaviour?

[jeromekelleher]

I think they're all still marked as samples (we probably OR the flags with TSK_NODE_IS_SAMPLE) but I don't know off the top of my head. Should be easy enough the check if there's any non-sample nodes in the ancestors ts?

[hyanwong]

Yeah, I did and it appears that they aren't marked as samples:

anc = tsinfer.generate_ancestors(samples)
anc_ts = tsinfer.match_ancestors(samples, anc, path_compression=False)
print(anc_ts.num_nodes, anc_ts.num_samples)

anc_ts = tsinfer.match_ancestors(samples, anc)
print(anc_ts.num_nodes, anc_ts.num_samples)

gives

6402 6402
7372 6402

[hyanwong]

As discussed with @jeromekelleher, we should probably only set the is_sample flag when writing out the final ancestors tree sequence. We don't actually use it internally anyway - it's just for ease of printing out haplotypes.