Skip to content

Commit 002b006

Browse files
jeromekelleherjeromekelleher
committed
Refine API, add tests and fix implementation error
1 parent f02781f commit 002b006

File tree

8 files changed

+209
-97
lines changed

8 files changed

+209
-97
lines changed

‎c/tests/test_trees.c

Lines changed: 67 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -331,7 +331,7 @@ verify_trees(tsk_treeseq_t *ts, tsk_size_t num_trees, tsk_id_t *parents)
331331
uint32_t mutation_index, site_index;
332332
tsk_size_t k, l, tree_sites_length;
333333
const tsk_site_t *sites = NULL;
334-
tsk_tree_t tree;
334+
tsk_tree_t tree, skip_tree;
335335
tsk_size_t num_edges;
336336
tsk_size_t num_nodes = tsk_treeseq_get_num_nodes(ts);
337337
tsk_size_t num_sites = tsk_treeseq_get_num_sites(ts);
@@ -340,6 +340,7 @@ verify_trees(tsk_treeseq_t *ts, tsk_size_t num_trees, tsk_id_t *parents)
340340

341341
ret = tsk_tree_init(&tree, ts, 0);
342342
CU_ASSERT_EQUAL(ret, 0);
343+
ret = tsk_tree_init(&skip_tree, ts, 0);
343344
CU_ASSERT_EQUAL(tsk_treeseq_get_num_trees(ts), num_trees);
344345

345346
CU_ASSERT_EQUAL(tree.index, -1);
@@ -372,6 +373,21 @@ verify_trees(tsk_treeseq_t *ts, tsk_size_t num_trees, tsk_id_t *parents)
372373
}
373374
site_index++;
374375
}
376+
/* Check the skip tree */
377+
ret = tsk_tree_first(&skip_tree);
378+
CU_ASSERT_EQUAL(ret, TSK_TREE_OK);
379+
ret = tsk_tree_seek(&skip_tree, breakpoints[j], TSK_SEEK_SKIP);
380+
CU_ASSERT_EQUAL(ret, 0);
381+
/* Calling print_state here also verifies the integrity of the tree */
382+
tsk_tree_print_state(&skip_tree, _devnull);
383+
check_trees_equal(&tree, &skip_tree);
384+
ret = tsk_tree_last(&skip_tree);
385+
CU_ASSERT_EQUAL(ret, TSK_TREE_OK);
386+
ret = tsk_tree_seek(&skip_tree, breakpoints[j], TSK_SEEK_SKIP);
387+
CU_ASSERT_EQUAL(ret, 0);
388+
tsk_tree_print_state(&skip_tree, _devnull);
389+
check_trees_equal(&tree, &skip_tree);
390+
375391
j++;
376392
}
377393
CU_ASSERT_EQUAL(ret, 0);
@@ -381,6 +397,7 @@ verify_trees(tsk_treeseq_t *ts, tsk_size_t num_trees, tsk_id_t *parents)
381397
CU_ASSERT_EQUAL(tsk_treeseq_get_sequence_length(ts), breakpoints[j]);
382398

383399
tsk_tree_free(&tree);
400+
tsk_tree_free(&skip_tree);
384401

385402
verify_tree_pos(ts, num_trees, parents);
386403
}
@@ -811,7 +828,8 @@ typedef struct {
811828
} sample_count_test_t;
812829

813830
static void
814-
verify_sample_counts(tsk_treeseq_t *ts, tsk_size_t num_tests, sample_count_test_t *tests)
831+
verify_sample_counts(tsk_treeseq_t *ts, tsk_size_t num_tests, sample_count_test_t *tests,
832+
tsk_flags_t seek_options)
815833
{
816834
int ret;
817835
tsk_size_t j, num_samples, n, k;
@@ -826,13 +844,9 @@ verify_sample_counts(tsk_treeseq_t *ts, tsk_size_t num_tests, sample_count_test_
826844

827845
ret = tsk_tree_init(&tree, ts, TSK_NO_SAMPLE_COUNTS);
828846
CU_ASSERT_EQUAL(ret, 0);
829-
ret = tsk_tree_first(&tree);
830-
CU_ASSERT_EQUAL_FATAL(ret, TSK_TREE_OK);
831847
for (j = 0; j < num_tests; j++) {
832-
while (tree.index < tests[j].tree_index) {
833-
ret = tsk_tree_next(&tree);
834-
CU_ASSERT_EQUAL_FATAL(ret, TSK_TREE_OK);
835-
}
848+
ret = tsk_tree_seek_index(&tree, tests[j].tree_index, seek_options);
849+
CU_ASSERT_EQUAL_FATAL(ret, 0);
836850
ret = tsk_tree_get_num_samples(&tree, tests[j].node, &num_samples);
837851
CU_ASSERT_EQUAL_FATAL(ret, 0);
838852
CU_ASSERT_EQUAL(tests[j].count, num_samples);
@@ -850,10 +864,8 @@ verify_sample_counts(tsk_treeseq_t *ts, tsk_size_t num_tests, sample_count_test_
850864
ret = tsk_tree_first(&tree);
851865
CU_ASSERT_EQUAL_FATAL(ret, TSK_TREE_OK);
852866
for (j = 0; j < num_tests; j++) {
853-
while (tree.index < tests[j].tree_index) {
854-
ret = tsk_tree_next(&tree);
855-
CU_ASSERT_EQUAL_FATAL(ret, TSK_TREE_OK);
856-
}
867+
ret = tsk_tree_seek_index(&tree, tests[j].tree_index, seek_options);
868+
CU_ASSERT_EQUAL_FATAL(ret, 0);
857869
ret = tsk_tree_get_num_samples(&tree, tests[j].node, &num_samples);
858870
CU_ASSERT_EQUAL_FATAL(ret, 0);
859871
CU_ASSERT_EQUAL(tests[j].count, num_samples);
@@ -872,10 +884,8 @@ verify_sample_counts(tsk_treeseq_t *ts, tsk_size_t num_tests, sample_count_test_
872884
ret = tsk_tree_first(&tree);
873885
CU_ASSERT_EQUAL_FATAL(ret, TSK_TREE_OK);
874886
for (j = 0; j < num_tests; j++) {
875-
while (tree.index < tests[j].tree_index) {
876-
ret = tsk_tree_next(&tree);
877-
CU_ASSERT_EQUAL_FATAL(ret, TSK_TREE_OK);
878-
}
887+
ret = tsk_tree_seek_index(&tree, tests[j].tree_index, seek_options);
888+
CU_ASSERT_EQUAL_FATAL(ret, 0);
879889
ret = tsk_tree_get_num_samples(&tree, tests[j].node, &num_samples);
880890
CU_ASSERT_EQUAL_FATAL(ret, 0);
881891
CU_ASSERT_EQUAL(tests[j].count, num_samples);
@@ -908,10 +918,8 @@ verify_sample_counts(tsk_treeseq_t *ts, tsk_size_t num_tests, sample_count_test_
908918
ret = tsk_tree_first(&tree);
909919
CU_ASSERT_EQUAL_FATAL(ret, TSK_TREE_OK);
910920
for (j = 0; j < num_tests; j++) {
911-
while (tree.index < tests[j].tree_index) {
912-
ret = tsk_tree_next(&tree);
913-
CU_ASSERT_EQUAL_FATAL(ret, TSK_TREE_OK);
914-
}
921+
ret = tsk_tree_seek_index(&tree, tests[j].tree_index, seek_options);
922+
CU_ASSERT_EQUAL_FATAL(ret, 0);
915923
ret = tsk_tree_get_num_samples(&tree, tests[j].node, &num_samples);
916924
CU_ASSERT_EQUAL_FATAL(ret, 0);
917925
CU_ASSERT_EQUAL(tests[j].count, num_samples);
@@ -1008,6 +1016,7 @@ verify_sample_sets(tsk_treeseq_t *ts)
10081016
{
10091017
int ret;
10101018
tsk_tree_t t;
1019+
tsk_id_t j;
10111020

10121021
ret = tsk_tree_init(&t, ts, TSK_SAMPLE_LISTS);
10131022
CU_ASSERT_EQUAL(ret, 0);
@@ -1021,6 +1030,20 @@ verify_sample_sets(tsk_treeseq_t *ts)
10211030
}
10221031
CU_ASSERT_EQUAL_FATAL(ret, 0);
10231032

1033+
for (j = 0; j < (tsk_id_t) tsk_treeseq_get_num_trees(ts); j++) {
1034+
ret = tsk_tree_first(&t);
1035+
CU_ASSERT_EQUAL_FATAL(ret, TSK_TREE_OK);
1036+
ret = tsk_tree_seek_index(&t, j, TSK_SEEK_SKIP);
1037+
CU_ASSERT_EQUAL_FATAL(ret, 0);
1038+
verify_sample_sets_for_tree(&t);
1039+
1040+
ret = tsk_tree_last(&t);
1041+
CU_ASSERT_EQUAL_FATAL(ret, TSK_TREE_OK);
1042+
ret = tsk_tree_seek_index(&t, j, TSK_SEEK_SKIP);
1043+
CU_ASSERT_EQUAL_FATAL(ret, 0);
1044+
verify_sample_sets_for_tree(&t);
1045+
}
1046+
10241047
tsk_tree_free(&t);
10251048
}
10261049

@@ -5870,7 +5893,8 @@ test_simple_sample_sets(void)
58705893

58715894
tsk_treeseq_from_text(&ts, 10, paper_ex_nodes, paper_ex_edges, NULL, NULL, NULL,
58725895
paper_ex_individuals, NULL, 0);
5873-
verify_sample_counts(&ts, num_tests, tests);
5896+
verify_sample_counts(&ts, num_tests, tests, 0);
5897+
verify_sample_counts(&ts, num_tests, tests, TSK_SEEK_SKIP);
58745898
verify_sample_sets(&ts);
58755899

58765900
tsk_treeseq_free(&ts);
@@ -5889,7 +5913,8 @@ test_nonbinary_sample_sets(void)
58895913

58905914
tsk_treeseq_from_text(&ts, 100, nonbinary_ex_nodes, nonbinary_ex_edges, NULL, NULL,
58915915
NULL, NULL, NULL, 0);
5892-
verify_sample_counts(&ts, num_tests, tests);
5916+
verify_sample_counts(&ts, num_tests, tests, 0);
5917+
verify_sample_counts(&ts, num_tests, tests, TSK_SEEK_SKIP);
58935918
verify_sample_sets(&ts);
58945919

58955920
tsk_treeseq_free(&ts);
@@ -5909,7 +5934,8 @@ test_internal_sample_sample_sets(void)
59095934

59105935
tsk_treeseq_from_text(&ts, 10, internal_sample_ex_nodes, internal_sample_ex_edges,
59115936
NULL, NULL, NULL, NULL, NULL, 0);
5912-
verify_sample_counts(&ts, num_tests, tests);
5937+
verify_sample_counts(&ts, num_tests, tests, 0);
5938+
verify_sample_counts(&ts, num_tests, tests, TSK_SEEK_SKIP);
59135939
verify_sample_sets(&ts);
59145940

59155941
tsk_treeseq_free(&ts);
@@ -6283,7 +6309,7 @@ test_multiroot_tree_traversal(void)
62836309
}
62846310

62856311
static void
6286-
test_seek_multi_tree(void)
6312+
verify_seek_multi_tree(tsk_flags_t seek_options)
62876313
{
62886314
int ret;
62896315
tsk_treeseq_t ts;
@@ -6299,29 +6325,29 @@ test_seek_multi_tree(void)
62996325
CU_ASSERT_EQUAL_FATAL(ret, 0);
63006326

63016327
for (j = 0; j < num_trees; j++) {
6302-
ret = tsk_tree_seek(&t, breakpoints[j], 0);
6328+
ret = tsk_tree_seek(&t, breakpoints[j], seek_options);
63036329
CU_ASSERT_EQUAL_FATAL(ret, 0);
63046330
CU_ASSERT_EQUAL_FATAL(t.index, j);
6305-
ret = tsk_tree_seek_index(&t, j, 0);
6331+
ret = tsk_tree_seek_index(&t, j, seek_options);
63066332
CU_ASSERT_EQUAL_FATAL(ret, 0);
63076333
CU_ASSERT_EQUAL_FATAL(t.index, j);
63086334
for (k = 0; k < num_trees; k++) {
6309-
ret = tsk_tree_seek(&t, breakpoints[k], 0);
6335+
ret = tsk_tree_seek(&t, breakpoints[k], seek_options);
63106336
CU_ASSERT_EQUAL_FATAL(ret, 0);
63116337
CU_ASSERT_EQUAL_FATAL(t.index, k);
6312-
ret = tsk_tree_seek_index(&t, k, 0);
6338+
ret = tsk_tree_seek_index(&t, k, seek_options);
63136339
CU_ASSERT_EQUAL_FATAL(ret, 0);
63146340
CU_ASSERT_EQUAL_FATAL(t.index, k);
63156341
}
63166342
}
63176343

6318-
ret = tsk_tree_seek(&t, 1.99999, 0);
6344+
ret = tsk_tree_seek(&t, 1.99999, seek_options);
63196345
CU_ASSERT_EQUAL_FATAL(ret, 0);
63206346
CU_ASSERT_EQUAL_FATAL(t.index, 0);
6321-
ret = tsk_tree_seek(&t, 6.99999, 0);
6347+
ret = tsk_tree_seek(&t, 6.99999, seek_options);
63226348
CU_ASSERT_EQUAL_FATAL(ret, 0);
63236349
CU_ASSERT_EQUAL_FATAL(t.index, 1);
6324-
ret = tsk_tree_seek(&t, 9.99999, 0);
6350+
ret = tsk_tree_seek(&t, 9.99999, seek_options);
63256351
CU_ASSERT_EQUAL_FATAL(ret, 0);
63266352
CU_ASSERT_EQUAL_FATAL(t.index, 2);
63276353

@@ -6331,7 +6357,7 @@ test_seek_multi_tree(void)
63316357
for (j = 0; j < num_trees; j++) {
63326358
ret = tsk_tree_init(&t, &ts, 0);
63336359
CU_ASSERT_EQUAL_FATAL(ret, 0);
6334-
ret = tsk_tree_seek(&t, breakpoints[j], 0);
6360+
ret = tsk_tree_seek(&t, breakpoints[j], seek_options);
63356361
CU_ASSERT_EQUAL_FATAL(ret, 0);
63366362
CU_ASSERT_EQUAL_FATAL(t.index, j);
63376363
tsk_tree_free(&t);
@@ -6341,12 +6367,12 @@ test_seek_multi_tree(void)
63416367
ret = tsk_tree_init(&t, &ts, 0);
63426368
CU_ASSERT_EQUAL_FATAL(ret, 0);
63436369
for (j = 0; j < num_trees; j++) {
6344-
ret = tsk_tree_seek(&t, 0, 0);
6370+
ret = tsk_tree_seek(&t, 0, seek_options);
63456371
CU_ASSERT_EQUAL_FATAL(ret, 0);
63466372
ret = tsk_tree_prev(&t);
63476373
CU_ASSERT_EQUAL_FATAL(ret, 0);
63486374
CU_ASSERT_EQUAL_FATAL(t.index, -1);
6349-
ret = tsk_tree_seek(&t, breakpoints[j], 0);
6375+
ret = tsk_tree_seek(&t, breakpoints[j], seek_options);
63506376
CU_ASSERT_EQUAL_FATAL(ret, 0);
63516377
CU_ASSERT_EQUAL_FATAL(t.index, j);
63526378
}
@@ -6355,6 +6381,13 @@ test_seek_multi_tree(void)
63556381
tsk_treeseq_free(&ts);
63566382
}
63576383

6384+
static void
6385+
test_seek_multi_tree(void)
6386+
{
6387+
verify_seek_multi_tree(0);
6388+
verify_seek_multi_tree(TSK_SEEK_SKIP);
6389+
}
6390+
63586391
static void
63596392
test_seek_errors(void)
63606393
{

‎c/tskit/trees.c

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6146,17 +6146,18 @@ tsk_tree_print_state(const tsk_tree_t *self, FILE *out)
61466146
fprintf(out, "left = %f\n", self->interval.left);
61476147
fprintf(out, "right = %f\n", self->interval.right);
61486148
fprintf(out, "index = %lld\n", (long long) self->index);
6149-
fprintf(out, "node\tparent\tlchild\trchild\tlsib\trsib");
6149+
fprintf(out, "num_edges = %d\n", (int) self->num_edges);
6150+
fprintf(out, "node\tedge\tparent\tlchild\trchild\tlsib\trsib");
61506151
if (self->options & TSK_SAMPLE_LISTS) {
61516152
fprintf(out, "\thead\ttail");
61526153
}
61536154
fprintf(out, "\n");
61546155

61556156
for (j = 0; j < self->num_nodes + 1; j++) {
6156-
fprintf(out, "%lld\t%lld\t%lld\t%lld\t%lld\t%lld", (long long) j,
6157-
(long long) self->parent[j], (long long) self->left_child[j],
6158-
(long long) self->right_child[j], (long long) self->left_sib[j],
6159-
(long long) self->right_sib[j]);
6157+
fprintf(out, "%lld\t%lld\t%lld\t%lld\t%lld\t%lld\t%lld", (long long) j,
6158+
(long long) self->edge[j], (long long) self->parent[j],
6159+
(long long) self->left_child[j], (long long) self->right_child[j],
6160+
(long long) self->left_sib[j], (long long) self->right_sib[j]);
61606161
if (self->options & TSK_SAMPLE_LISTS) {
61616162
fprintf(out, "\t%lld\t%lld\t", (long long) self->left_sample[j],
61626163
(long long) self->right_sample[j]);
@@ -6284,7 +6285,8 @@ tsk_tree_remove_root(tsk_tree_t *self, tsk_id_t root, tsk_id_t *restrict parent)
62846285
}
62856286

62866287
static void
6287-
tsk_tree_remove_edge(tsk_tree_t *self, tsk_id_t p, tsk_id_t c)
6288+
tsk_tree_remove_edge(
6289+
tsk_tree_t *self, tsk_id_t p, tsk_id_t c, tsk_id_t TSK_UNUSED(edge_id))
62886290
{
62896291
tsk_id_t *restrict parent = self->parent;
62906292
tsk_size_t *restrict num_samples = self->num_samples;
@@ -6425,7 +6427,7 @@ tsk_tree_next(tsk_tree_t *self)
64256427
if (valid) {
64266428
for (j = tree_pos.out.start; j != tree_pos.out.stop; j++) {
64276429
e = tree_pos.out.order[j];
6428-
tsk_tree_remove_edge(self, edge_parent[e], edge_child[e]);
6430+
tsk_tree_remove_edge(self, edge_parent[e], edge_child[e], e);
64296431
}
64306432

64316433
for (j = tree_pos.in.start; j != tree_pos.in.stop; j++) {
@@ -6457,7 +6459,7 @@ tsk_tree_prev(tsk_tree_t *self)
64576459
if (valid) {
64586460
for (j = tree_pos.out.start; j != tree_pos.out.stop; j--) {
64596461
e = tree_pos.out.order[j];
6460-
tsk_tree_remove_edge(self, edge_parent[e], edge_child[e]);
6462+
tsk_tree_remove_edge(self, edge_parent[e], edge_child[e], e);
64616463
}
64626464

64636465
for (j = tree_pos.in.start; j != tree_pos.in.stop; j--) {
@@ -6556,9 +6558,9 @@ tsk_tree_seek_forward(tsk_tree_t *self, tsk_id_t index)
65566558
for (j = tree_pos.out.start; j != tree_pos.out.stop; j++) {
65576559
e = tree_pos.out.order[j];
65586560
e_left = edge_left[e];
6559-
if (e_left <= old_right) {
6561+
if (e_left < old_right) {
65606562
tsk_bug_assert(edge_parent[e] != TSK_NULL);
6561-
tsk_tree_remove_edge(self, edge_parent[e], edge_child[e]);
6563+
tsk_tree_remove_edge(self, edge_parent[e], edge_child[e], e);
65626564
}
65636565
tsk_bug_assert(e_left < interval_left);
65646566
}
@@ -6600,7 +6602,7 @@ tsk_tree_seek_backward(tsk_tree_t *self, tsk_id_t index)
66006602
e_right = edge_right[e];
66016603
if (e_right >= old_right) {
66026604
tsk_bug_assert(edge_parent[e] != TSK_NULL);
6603-
tsk_tree_remove_edge(self, edge_parent[e], edge_child[e]);
6605+
tsk_tree_remove_edge(self, edge_parent[e], edge_child[e], e);
66046606
}
66056607
tsk_bug_assert(e_right > interval_right);
66066608
}
@@ -6709,7 +6711,7 @@ tsk_tree_seek(tsk_tree_t *self, double x, tsk_flags_t options)
67096711
if (self->index == -1) {
67106712
ret = tsk_tree_seek_from_null(self, x, options);
67116713
} else {
6712-
if (options & TSK_TREE_SEEK_ENABLE_SKIPPING) {
6714+
if (options & TSK_SEEK_SKIP) {
67136715
ret = tsk_tree_seek_skip(self, x);
67146716
} else {
67156717
ret = tsk_tree_seek_linear(self, x);

‎c/tskit/trees.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1275,7 +1275,7 @@ int tsk_tree_copy(const tsk_tree_t *self, tsk_tree_t *dest, tsk_flags_t options)
12751275
12761276
@ingroup TREE_API_SEEKING_GROUP
12771277
*/
1278-
#define TSK_TREE_SEEK_ENABLE_SKIPPING (1 << 0)
1278+
#define TSK_SEEK_SKIP (1 << 0)
12791279

12801280
/**
12811281
@brief Seek to the first tree in the sequence.
@@ -1286,7 +1286,7 @@ tree sequence.
12861286
@endrst
12871287
12881288
@param self A pointer to an initialised tsk_tree_t object.
1289-
@return Return on success; or a negative value if an error occurs.
1289+
@return Return TSK_TREE_OK on success; or a negative value if an error occurs.
12901290
*/
12911291
int tsk_tree_first(tsk_tree_t *self);
12921292

@@ -1384,13 +1384,13 @@ Seeking to a position currently covered by the tree is
13841384
a constant time operation.
13851385
13861386
Seeking to a position from a non-null tree use a linear time
1387-
algorithm by default, unless the option :c:macro:`TSK_TREE_SEEK_ENABLE_SKIPPING`
1387+
algorithm by default, unless the option :c:macro:`TSK_SEEK_SKIP`
13881388
is specified. In this case, a faster algorithm is employed which skips
13891389
to the target tree by removing and adding the minimal number of edges
13901390
possible. However, this approach does not guarantee that edges are
13911391
inserted and removed in time-sorted order.
13921392
1393-
.. warning:: Using the :c:macro:`TSK_TREE_SEEK_ENABLE_SKIPPING` option
1393+
.. warning:: Using the :c:macro:`TSK_SEEK_SKIP` option
13941394
may lead to edges not being inserted or removed in time-sorted order.
13951395
13961396
@endrst

0 commit comments

Comments
 (0)