Tskit: [C API] Document tsk_table_collection_check_integrity()

[bhaller]

In the course of tracking down a bug in SLiM having to do with an uncaught (by SLiM) overflow of a tskit table (didn't check the pesky return value!), @molpopgen wrote a little snippet of debugging code, which I modified into this form:

        std::size_t debug_edge_index = 0;

        for( ; debug_edge_index < tables_.edges.num_rows ; ++debug_edge_index)
        {
            if ((tables_.edges.parent[debug_edge_index] >= tables_.nodes.num_rows) || (tables_.edges.parent[debug_edge_index] < 0))
            {
                std::cout << "parent index out of range: " << tables_.edges.parent[debug_edge_index] << ", " << tables_.nodes.num_rows << std::endl;
                break;
            }
            if ((tables_.edges.child[debug_edge_index] >= tables_.nodes.num_rows) || (tables_.edges.child[debug_edge_index] < 0))
            {
                std::cout << "child index out of range: " << tables_.edges.child[debug_edge_index] << ", " << tables_.nodes.num_rows << std::endl;
                break;
            }
        }

It caught the problem and provided the key information needed to track down the bug. It's not rocket science, obviously, but it got me to thinking that it would be useful for tskit to have a "consistency check" function that would check all sorts of things about a table collection: that all indices are within range, that things that ought to correspond with each other do correspond with each other, etc. That the table collection is, in every respect one can think of, well-formed. Client code like SLiM, fwdpp, etc., could call that check as often as desired to make sure that nothing untoward has occurred. I'll tag @petrelharp here as well since he was in the previous discussion / bug-hunt. :->

[bhaller]

@petrelharp has pointed out tsk_table_collection_check_integrity() to me, so it looks like you're way ahead of me. :-> Clearly SLiM ought to be calling this function when in DEBUG mode! So , closing this bug now, sorry for the noise. :->

[bhaller]

(Although I do see that it's listed as "undocumented"; does that mean it's to be considered private API, not to be used by clients? If so, then maybe this issue should be reopened, and considered a request for this API to be made public...

[molpopgen]

it is public (non-static), but it could be handy to write the doxygen block so that it ends up in the manual.

[molpopgen]

FWIW, I just wrote this a few minutes ago for something I'm working on. The only trick is that I'm using std::unique_ptr with a custom deleter to manage the lifetime of the table collection:

static void
handle_tskit_return_code(int code)
{
    std::ostringstream o;
    o << tsk_strerror(code);
    if (code != 0)
        {
            throw std::runtime_error(o.str());
        }
}

static void
validate_stitched_tables(const table_collection_ptr& tables)
// Relatively expensive checks
{
    int rv = tsk_table_collection_check_integrity(tables.get(), TSK_CHECK_EDGE_ORDERING);
    handle_tskit_return_code(rv);
    rv = tsk_table_collection_check_integrity(tables.get(), TSK_CHECK_OFFSETS);
    handle_tskit_return_code(rv);
}

[molpopgen]

btw, that "relatively expensive" comment isn't saying it'll show up in a profile--it gets run during each simplification normally and one hardly notices. That's just a note to self to wrap it in an ifdef/endif block to get it into debug only mode later.

[jeromekelleher]

I've reopened this issue and renamed it @bhaller - you're right, this should be properly documented as it's a really helpful debugging tool.