avoid possible infinite loop in canonicalize

[petrelharp]

Looking at #1192 I realized there was potential for an infinite loop in the case of bad mutation parent input. Whoops! Here's a fix.

[codecov]

Codecov Report

Merging #1198 (9ed3cc4) into main (658896c) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #1198      +/-   ##
==========================================
+ Coverage   93.70%   93.71%   +0.01%     
==========================================
  Files          26       26              
  Lines       21466    21472       +6     
  Branches      904      904              
==========================================
+ Hits        20114    20122       +8     
+ Misses       1314     1312       -2     
  Partials       38       38              

Flag	Coverage Δ
c-tests	92.48% <100.00%> (+0.02%)	⬆️
lwt-tests	92.97% <ø> (ø)
python-c-tests	94.90% <ø> (ø)
python-tests	98.63% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
c/tskit/core.h	100.00% <ø> (ø)
c/tskit/core.c	96.99% <100.00%> (+0.02%)	⬆️
c/tskit/tables.c	90.79% <100.00%> (+0.03%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 658896c...9ed3cc4. Read the comment docs.

[petrelharp]

Whoops, this doesn't fix it. Consider 0->1, 1->2, 2->1. Huh, tricky.

[jeromekelleher]

I guess we have to sort first so that parents come before children, and then check for inconsistencies? (Like #1197?)

[petrelharp]

The problem occurs while computing the number of descendant mutations, which is the key that allows us to sort so parents come before children if times are equal (or, unknown).

This is the problem of detecting cycles in a directed graph; I'll have a look at algorithms for that.

[petrelharp]

Ok, this will correctly check things now. I think it's totally good enough.

[petrelharp]

Wait, never mind - not right yet.

[petrelharp]

Ok, for reals now!

I think to do this in a clever way we'd need to build the trees-of-mutations and then traverse them in an ordered fashion, which seems overkill here, since in general we don't expect a lot of mutations per site. I'm just checking if there's a loop in the boneheaded way: by checking if we think any mutation has more than num_mutations descendants.

[jeromekelleher]

LGTM!

[jeromekelleher]

This is clever - detect a loop by letting it loop until we know it's definitely gone too far. Who cares if it's not the most efficient possible way of detecting the loop, it's an error anyway. Very elegant!