Skip to content

Commit 46cc619

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

File tree

8 files changed

+200
-97
lines changed

8 files changed

+200
-97
lines changed

‎c/tests/test_trees.c

Lines changed: 68 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,22 @@ 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+
/* printf("Call first %p\n", (void *) &skip_tree); */
378+
ret = tsk_tree_first(&skip_tree);
379+
/* tsk_tree_print_state(&skip_tree, stdout); */
380+
CU_ASSERT_EQUAL(ret, TSK_TREE_OK);
381+
/* printf("Call seek %p\n", (void *) &skip_tree); */
382+
ret = tsk_tree_seek(&skip_tree, breakpoints[j], TSK_SEEK_SKIP);
383+
CU_ASSERT_EQUAL(ret, 0);
384+
/* tsk_tree_print_state(&skip_tree, _devnull); */
385+
check_trees_equal(&tree, &skip_tree);
386+
ret = tsk_tree_last(&skip_tree);
387+
CU_ASSERT_EQUAL(ret, TSK_TREE_OK);
388+
ret = tsk_tree_seek(&skip_tree, breakpoints[j], TSK_SEEK_SKIP);
389+
CU_ASSERT_EQUAL(ret, 0);
390+
/* tsk_tree_print_state(&skip_tree, _devnull); */
391+
check_trees_equal(&tree, &skip_tree);
375392
j++;
376393
}
377394
CU_ASSERT_EQUAL(ret, 0);
@@ -381,6 +398,7 @@ verify_trees(tsk_treeseq_t *ts, tsk_size_t num_trees, tsk_id_t *parents)
381398
CU_ASSERT_EQUAL(tsk_treeseq_get_sequence_length(ts), breakpoints[j]);
382399

383400
tsk_tree_free(&tree);
401+
tsk_tree_free(&skip_tree);
384402

385403
verify_tree_pos(ts, num_trees, parents);
386404
}
@@ -811,7 +829,8 @@ typedef struct {
811829
} sample_count_test_t;
812830

813831
static void
814-
verify_sample_counts(tsk_treeseq_t *ts, tsk_size_t num_tests, sample_count_test_t *tests)
832+
verify_sample_counts(tsk_treeseq_t *ts, tsk_size_t num_tests, sample_count_test_t *tests,
833+
tsk_flags_t seek_options)
815834
{
816835
int ret;
817836
tsk_size_t j, num_samples, n, k;
@@ -826,13 +845,9 @@ verify_sample_counts(tsk_treeseq_t *ts, tsk_size_t num_tests, sample_count_test_
826845

827846
ret = tsk_tree_init(&tree, ts, TSK_NO_SAMPLE_COUNTS);
828847
CU_ASSERT_EQUAL(ret, 0);
829-
ret = tsk_tree_first(&tree);
830-
CU_ASSERT_EQUAL_FATAL(ret, TSK_TREE_OK);
831848
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-
}
849+
ret = tsk_tree_seek_index(&tree, tests[j].tree_index, seek_options);
850+
CU_ASSERT_EQUAL_FATAL(ret, 0);
836851
ret = tsk_tree_get_num_samples(&tree, tests[j].node, &num_samples);
837852
CU_ASSERT_EQUAL_FATAL(ret, 0);
838853
CU_ASSERT_EQUAL(tests[j].count, num_samples);
@@ -850,10 +865,8 @@ verify_sample_counts(tsk_treeseq_t *ts, tsk_size_t num_tests, sample_count_test_
850865
ret = tsk_tree_first(&tree);
851866
CU_ASSERT_EQUAL_FATAL(ret, TSK_TREE_OK);
852867
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-
}
868+
ret = tsk_tree_seek_index(&tree, tests[j].tree_index, seek_options);
869+
CU_ASSERT_EQUAL_FATAL(ret, 0);
857870
ret = tsk_tree_get_num_samples(&tree, tests[j].node, &num_samples);
858871
CU_ASSERT_EQUAL_FATAL(ret, 0);
859872
CU_ASSERT_EQUAL(tests[j].count, num_samples);
@@ -872,10 +885,8 @@ verify_sample_counts(tsk_treeseq_t *ts, tsk_size_t num_tests, sample_count_test_
872885
ret = tsk_tree_first(&tree);
873886
CU_ASSERT_EQUAL_FATAL(ret, TSK_TREE_OK);
874887
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-
}
888+
ret = tsk_tree_seek_index(&tree, tests[j].tree_index, seek_options);
889+
CU_ASSERT_EQUAL_FATAL(ret, 0);
879890
ret = tsk_tree_get_num_samples(&tree, tests[j].node, &num_samples);
880891
CU_ASSERT_EQUAL_FATAL(ret, 0);
881892
CU_ASSERT_EQUAL(tests[j].count, num_samples);
@@ -908,10 +919,8 @@ verify_sample_counts(tsk_treeseq_t *ts, tsk_size_t num_tests, sample_count_test_
908919
ret = tsk_tree_first(&tree);
909920
CU_ASSERT_EQUAL_FATAL(ret, TSK_TREE_OK);
910921
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-
}
922+
ret = tsk_tree_seek_index(&tree, tests[j].tree_index, seek_options);
923+
CU_ASSERT_EQUAL_FATAL(ret, 0);
915924
ret = tsk_tree_get_num_samples(&tree, tests[j].node, &num_samples);
916925
CU_ASSERT_EQUAL_FATAL(ret, 0);
917926
CU_ASSERT_EQUAL(tests[j].count, num_samples);
@@ -1008,6 +1017,7 @@ verify_sample_sets(tsk_treeseq_t *ts)
10081017
{
10091018
int ret;
10101019
tsk_tree_t t;
1020+
tsk_id_t j;
10111021

10121022
ret = tsk_tree_init(&t, ts, TSK_SAMPLE_LISTS);
10131023
CU_ASSERT_EQUAL(ret, 0);
@@ -1021,6 +1031,20 @@ verify_sample_sets(tsk_treeseq_t *ts)
10211031
}
10221032
CU_ASSERT_EQUAL_FATAL(ret, 0);
10231033

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

@@ -5870,7 +5894,8 @@ test_simple_sample_sets(void)
58705894

58715895
tsk_treeseq_from_text(&ts, 10, paper_ex_nodes, paper_ex_edges, NULL, NULL, NULL,
58725896
paper_ex_individuals, NULL, 0);
5873-
verify_sample_counts(&ts, num_tests, tests);
5897+
verify_sample_counts(&ts, num_tests, tests, 0);
5898+
verify_sample_counts(&ts, num_tests, tests, TSK_SEEK_SKIP);
58745899
verify_sample_sets(&ts);
58755900

58765901
tsk_treeseq_free(&ts);
@@ -5889,7 +5914,8 @@ test_nonbinary_sample_sets(void)
58895914

58905915
tsk_treeseq_from_text(&ts, 100, nonbinary_ex_nodes, nonbinary_ex_edges, NULL, NULL,
58915916
NULL, NULL, NULL, 0);
5892-
verify_sample_counts(&ts, num_tests, tests);
5917+
verify_sample_counts(&ts, num_tests, tests, 0);
5918+
verify_sample_counts(&ts, num_tests, tests, TSK_SEEK_SKIP);
58935919
verify_sample_sets(&ts);
58945920

58955921
tsk_treeseq_free(&ts);
@@ -5909,7 +5935,8 @@ test_internal_sample_sample_sets(void)
59095935

59105936
tsk_treeseq_from_text(&ts, 10, internal_sample_ex_nodes, internal_sample_ex_edges,
59115937
NULL, NULL, NULL, NULL, NULL, 0);
5912-
verify_sample_counts(&ts, num_tests, tests);
5938+
verify_sample_counts(&ts, num_tests, tests, 0);
5939+
verify_sample_counts(&ts, num_tests, tests, TSK_SEEK_SKIP);
59135940
verify_sample_sets(&ts);
59145941

59155942
tsk_treeseq_free(&ts);
@@ -6283,7 +6310,7 @@ test_multiroot_tree_traversal(void)
62836310
}
62846311

62856312
static void
6286-
test_seek_multi_tree(void)
6313+
verify_seek_multi_tree(tsk_flags_t seek_options)
62876314
{
62886315
int ret;
62896316
tsk_treeseq_t ts;
@@ -6299,29 +6326,29 @@ test_seek_multi_tree(void)
62996326
CU_ASSERT_EQUAL_FATAL(ret, 0);
63006327

63016328
for (j = 0; j < num_trees; j++) {
6302-
ret = tsk_tree_seek(&t, breakpoints[j], 0);
6329+
ret = tsk_tree_seek(&t, breakpoints[j], seek_options);
63036330
CU_ASSERT_EQUAL_FATAL(ret, 0);
63046331
CU_ASSERT_EQUAL_FATAL(t.index, j);
6305-
ret = tsk_tree_seek_index(&t, j, 0);
6332+
ret = tsk_tree_seek_index(&t, j, seek_options);
63066333
CU_ASSERT_EQUAL_FATAL(ret, 0);
63076334
CU_ASSERT_EQUAL_FATAL(t.index, j);
63086335
for (k = 0; k < num_trees; k++) {
6309-
ret = tsk_tree_seek(&t, breakpoints[k], 0);
6336+
ret = tsk_tree_seek(&t, breakpoints[k], seek_options);
63106337
CU_ASSERT_EQUAL_FATAL(ret, 0);
63116338
CU_ASSERT_EQUAL_FATAL(t.index, k);
6312-
ret = tsk_tree_seek_index(&t, k, 0);
6339+
ret = tsk_tree_seek_index(&t, k, seek_options);
63136340
CU_ASSERT_EQUAL_FATAL(ret, 0);
63146341
CU_ASSERT_EQUAL_FATAL(t.index, k);
63156342
}
63166343
}
63176344

6318-
ret = tsk_tree_seek(&t, 1.99999, 0);
6345+
ret = tsk_tree_seek(&t, 1.99999, seek_options);
63196346
CU_ASSERT_EQUAL_FATAL(ret, 0);
63206347
CU_ASSERT_EQUAL_FATAL(t.index, 0);
6321-
ret = tsk_tree_seek(&t, 6.99999, 0);
6348+
ret = tsk_tree_seek(&t, 6.99999, seek_options);
63226349
CU_ASSERT_EQUAL_FATAL(ret, 0);
63236350
CU_ASSERT_EQUAL_FATAL(t.index, 1);
6324-
ret = tsk_tree_seek(&t, 9.99999, 0);
6351+
ret = tsk_tree_seek(&t, 9.99999, seek_options);
63256352
CU_ASSERT_EQUAL_FATAL(ret, 0);
63266353
CU_ASSERT_EQUAL_FATAL(t.index, 2);
63276354

@@ -6331,7 +6358,7 @@ test_seek_multi_tree(void)
63316358
for (j = 0; j < num_trees; j++) {
63326359
ret = tsk_tree_init(&t, &ts, 0);
63336360
CU_ASSERT_EQUAL_FATAL(ret, 0);
6334-
ret = tsk_tree_seek(&t, breakpoints[j], 0);
6361+
ret = tsk_tree_seek(&t, breakpoints[j], seek_options);
63356362
CU_ASSERT_EQUAL_FATAL(ret, 0);
63366363
CU_ASSERT_EQUAL_FATAL(t.index, j);
63376364
tsk_tree_free(&t);
@@ -6341,12 +6368,12 @@ test_seek_multi_tree(void)
63416368
ret = tsk_tree_init(&t, &ts, 0);
63426369
CU_ASSERT_EQUAL_FATAL(ret, 0);
63436370
for (j = 0; j < num_trees; j++) {
6344-
ret = tsk_tree_seek(&t, 0, 0);
6371+
ret = tsk_tree_seek(&t, 0, seek_options);
63456372
CU_ASSERT_EQUAL_FATAL(ret, 0);
63466373
ret = tsk_tree_prev(&t);
63476374
CU_ASSERT_EQUAL_FATAL(ret, 0);
63486375
CU_ASSERT_EQUAL_FATAL(t.index, -1);
6349-
ret = tsk_tree_seek(&t, breakpoints[j], 0);
6376+
ret = tsk_tree_seek(&t, breakpoints[j], seek_options);
63506377
CU_ASSERT_EQUAL_FATAL(ret, 0);
63516378
CU_ASSERT_EQUAL_FATAL(t.index, j);
63526379
}
@@ -6355,6 +6382,13 @@ test_seek_multi_tree(void)
63556382
tsk_treeseq_free(&ts);
63566383
}
63576384

6385+
static void
6386+
test_seek_multi_tree(void)
6387+
{
6388+
verify_seek_multi_tree(0);
6389+
verify_seek_multi_tree(TSK_SEEK_SKIP);
6390+
}
6391+
63586392
static void
63596393
test_seek_errors(void)
63606394
{

‎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)