From 5858323fd5a24e559de6fd587fee5df2f391aa4a Mon Sep 17 00:00:00 2001 From: Jerome Kelleher Date: Sun, 14 Nov 2021 19:48:16 +0000 Subject: [PATCH] Rename tsk_tree_t.left/right to interval.left Closes #1686 --- c/CHANGELOG.rst | 4 +++ c/tests/test_trees.c | 40 +++++++++++------------ c/tskit/haplotype_matching.c | 6 ++-- c/tskit/trees.c | 61 +++++++++++++++++++----------------- c/tskit/trees.h | 14 ++++++--- python/_tskitmodule.c | 4 +-- 6 files changed, 70 insertions(+), 59 deletions(-) diff --git a/c/CHANGELOG.rst b/c/CHANGELOG.rst index fb07e77b09..e15bea52c2 100644 --- a/c/CHANGELOG.rst +++ b/c/CHANGELOG.rst @@ -26,6 +26,10 @@ member (which may lead to more concise code). (:user:`jeromekelleher`, :issue:`1796`, :pr:`1862`) +- Rename ``tsk_tree_t.left`` and ``tsk_tree_t.right`` members to + ``tsk_tree_t.interval.left`` and ``tsk_tree_t.interval.right`` respectively. + (:user:`jeromekelleher`, :issue:`1686`, :pr:`1913`) + **Features** diff --git a/c/tests/test_trees.c b/c/tests/test_trees.c index 047a005568..13bc724eed 100644 --- a/c/tests/test_trees.c +++ b/c/tests/test_trees.c @@ -42,8 +42,8 @@ check_trees_equal(tsk_tree_t *self, tsk_tree_t *other) CU_ASSERT_FATAL(self->tree_sequence == other->tree_sequence); CU_ASSERT_FATAL(self->index == other->index); - CU_ASSERT_FATAL(self->left == other->left); - CU_ASSERT_FATAL(self->right == other->right); + CU_ASSERT_FATAL(self->interval.left == other->interval.left); + CU_ASSERT_FATAL(self->interval.right == other->interval.right); CU_ASSERT_FATAL(self->sites_length == other->sites_length); CU_ASSERT_FATAL(self->sites == other->sites); CU_ASSERT_FATAL(self->samples == other->samples); @@ -202,7 +202,7 @@ verify_trees(tsk_treeseq_t *ts, tsk_size_t num_trees, tsk_id_t *parents) CU_ASSERT_EQUAL(j, (tsk_id_t) tree.index); tsk_tree_print_state(&tree, _devnull); /* tsk_tree_print_state(&tree, stdout); */ - CU_ASSERT_EQUAL(tree.left, breakpoints[j]); + CU_ASSERT_EQUAL(tree.interval.left, breakpoints[j]); num_edges = 0; for (u = 0; u < (tsk_id_t) num_nodes; u++) { ret = tsk_tree_get_parent(&tree, u, &v); @@ -253,8 +253,8 @@ get_tree_list(tsk_treeseq_t *ts) CU_ASSERT_EQUAL_FATAL(ret, 0); check_trees_equal(&trees[t.index], &t); /* Make sure the left and right coordinates are also OK */ - CU_ASSERT_EQUAL(trees[t.index].left, t.left); - CU_ASSERT_EQUAL(trees[t.index].right, t.right); + CU_ASSERT_EQUAL(trees[t.index].interval.left, t.interval.left); + CU_ASSERT_EQUAL(trees[t.index].interval.right, t.interval.right); } CU_ASSERT_EQUAL_FATAL(ret, 0); ret = tsk_tree_free(&t); @@ -445,8 +445,8 @@ verify_tree_diffs(tsk_treeseq_t *ts, tsk_flags_t options) for (j = 0; j < num_nodes; j++) { CU_ASSERT_EQUAL(parent[j], tree.parent[j]); } - CU_ASSERT_EQUAL(tree.left, lft); - CU_ASSERT_EQUAL(tree.right, rgt); + CU_ASSERT_EQUAL(tree.interval.left, lft); + CU_ASSERT_EQUAL(tree.interval.right, rgt); valid_tree = tsk_tree_next(&tree); if (num_trees < tsk_treeseq_get_num_trees(ts)) { CU_ASSERT_EQUAL(valid_tree, 1); @@ -600,7 +600,7 @@ verify_simplify_properties(tsk_treeseq_t *ts, tsk_treeseq_t *subset, total_sites = 0; while (1) { - while (full_tree.right <= subset_tree.right) { + while (full_tree.interval.right <= subset_tree.interval.right) { for (j = 0; j < num_samples; j++) { for (k = j + 1; k < num_samples; k++) { ret = tsk_tree_get_mrca(&full_tree, samples[j], samples[k], &mrca1); @@ -626,8 +626,8 @@ verify_simplify_properties(tsk_treeseq_t *ts, tsk_treeseq_t *subset, CU_ASSERT_EQUAL_FATAL(ret, 0); for (j = 0; j < tree_sites_length; j++) { - CU_ASSERT(subset_tree.left <= tree_sites[j].position); - CU_ASSERT(tree_sites[j].position < subset_tree.right); + CU_ASSERT(subset_tree.interval.left <= tree_sites[j].position); + CU_ASSERT(tree_sites[j].position < subset_tree.interval.right); for (k = 0; k < tree_sites[j].mutations_length; k++) { ret = tsk_tree_get_parent( &subset_tree, tree_sites[j].mutations[k].node, &u); @@ -5851,8 +5851,8 @@ test_empty_tree_kc(void) ret = tsk_tree_first(&t); CU_ASSERT_EQUAL_FATAL(ret, 1); CU_ASSERT_EQUAL_FATAL(tsk_tree_get_left_root(&t), TSK_NULL); - CU_ASSERT_EQUAL_FATAL(t.left, 0); - CU_ASSERT_EQUAL_FATAL(t.right, 1); + CU_ASSERT_EQUAL_FATAL(t.interval.left, 0); + CU_ASSERT_EQUAL_FATAL(t.interval.right, 1); CU_ASSERT_EQUAL_FATAL(t.parent[0], TSK_NULL); CU_ASSERT_EQUAL_FATAL(t.left_child[0], TSK_NULL); CU_ASSERT_EQUAL_FATAL(t.right_child[0], TSK_NULL); @@ -6717,8 +6717,8 @@ test_empty_tree_sequence(void) ret = tsk_tree_first(&t); CU_ASSERT_EQUAL_FATAL(ret, 1); CU_ASSERT_EQUAL_FATAL(tsk_tree_get_left_root(&t), TSK_NULL); - CU_ASSERT_EQUAL_FATAL(t.left, 0); - CU_ASSERT_EQUAL_FATAL(t.right, 1); + CU_ASSERT_EQUAL_FATAL(t.interval.left, 0); + CU_ASSERT_EQUAL_FATAL(t.interval.right, 1); CU_ASSERT_EQUAL_FATAL(t.num_edges, 0); CU_ASSERT_EQUAL_FATAL(tsk_tree_get_parent(&t, 0, &v), 0); CU_ASSERT_EQUAL_FATAL(v, TSK_NULL); @@ -6730,8 +6730,8 @@ test_empty_tree_sequence(void) ret = tsk_tree_last(&t); CU_ASSERT_EQUAL_FATAL(ret, 1); CU_ASSERT_EQUAL_FATAL(tsk_tree_get_left_root(&t), TSK_NULL); - CU_ASSERT_EQUAL_FATAL(t.left, 0); - CU_ASSERT_EQUAL_FATAL(t.right, 1); + CU_ASSERT_EQUAL_FATAL(t.interval.left, 0); + CU_ASSERT_EQUAL_FATAL(t.interval.right, 1); CU_ASSERT_EQUAL_FATAL(tsk_tree_get_parent(&t, 1, &v), TSK_ERR_NODE_OUT_OF_BOUNDS); tsk_tree_free(&t); @@ -6774,8 +6774,8 @@ test_zero_edges(void) CU_ASSERT_EQUAL_FATAL(ret, 0); ret = tsk_tree_first(&t); CU_ASSERT_EQUAL_FATAL(ret, 1); - CU_ASSERT_EQUAL(t.left, 0); - CU_ASSERT_EQUAL(t.right, 2); + CU_ASSERT_EQUAL(t.interval.left, 0); + CU_ASSERT_EQUAL(t.interval.right, 2); CU_ASSERT_EQUAL(t.num_edges, 0); CU_ASSERT_EQUAL(t.parent[0], TSK_NULL); CU_ASSERT_EQUAL(t.parent[1], TSK_NULL); @@ -6789,8 +6789,8 @@ test_zero_edges(void) CU_ASSERT_EQUAL_FATAL(ret, 0); ret = tsk_tree_last(&t); CU_ASSERT_EQUAL_FATAL(ret, 1); - CU_ASSERT_EQUAL(t.left, 0); - CU_ASSERT_EQUAL(t.right, 2); + CU_ASSERT_EQUAL(t.interval.left, 0); + CU_ASSERT_EQUAL(t.interval.right, 2); CU_ASSERT_EQUAL(t.parent[0], TSK_NULL); CU_ASSERT_EQUAL(t.parent[1], TSK_NULL); CU_ASSERT_EQUAL(tsk_tree_get_left_root(&t), 0); diff --git a/c/tskit/haplotype_matching.c b/c/tskit/haplotype_matching.c index f75638979d..48f56e200b 100644 --- a/c/tskit/haplotype_matching.c +++ b/c/tskit/haplotype_matching.c @@ -1570,14 +1570,14 @@ tsk_viterbi_matrix_traceback( if (ret != 0) { goto out; } - while (tree.left > site.position) { + while (tree.interval.left > site.position) { ret = tsk_tree_prev(&tree); if (ret < 0) { goto out; } } - tsk_bug_assert(tree.left <= site.position); - tsk_bug_assert(site.position < tree.right); + tsk_bug_assert(tree.interval.left <= site.position); + tsk_bug_assert(site.position < tree.interval.right); /* Fill in the recombination tree */ rr_record_tmp = rr_record; diff --git a/c/tskit/trees.c b/c/tskit/trees.c index ad6bf246ee..48e59105a0 100644 --- a/c/tskit/trees.c +++ b/c/tskit/trees.c @@ -3504,8 +3504,7 @@ tsk_tree_copy(const tsk_tree_t *self, tsk_tree_t *dest, tsk_flags_t options) ret = TSK_ERR_BAD_PARAM_VALUE; goto out; } - dest->left = self->left; - dest->right = self->right; + dest->interval = self->interval; dest->left_index = self->left_index; dest->right_index = self->right_index; dest->direction = self->direction; @@ -3927,8 +3926,8 @@ tsk_tree_check_state(const tsk_tree_t *self) } for (j = 0; j < self->sites_length; j++) { site = self->sites[j]; - tsk_bug_assert(self->left <= site.position); - tsk_bug_assert(site.position < self->right); + tsk_bug_assert(self->interval.left <= site.position); + tsk_bug_assert(site.position < self->interval.right); } if (!(self->options & TSK_NO_SAMPLE_COUNTS)) { @@ -3966,8 +3965,8 @@ tsk_tree_print_state(const tsk_tree_t *self, FILE *out) fprintf(out, "Tree state:\n"); fprintf(out, "options = %d\n", self->options); fprintf(out, "root_threshold = %lld\n", (long long) self->root_threshold); - fprintf(out, "left = %f\n", self->left); - fprintf(out, "right = %f\n", self->right); + fprintf(out, "left = %f\n", self->interval.left); + fprintf(out, "right = %f\n", self->interval.right); fprintf(out, "index = %lld\n", (long long) self->index); fprintf(out, "node\tparent\tlchild\trchild\tlsib\trsib"); if (self->options & TSK_SAMPLE_LISTS) { @@ -4205,9 +4204,9 @@ tsk_tree_advance(tsk_tree_t *self, int direction, const double *restrict out_bre double x; if (direction == TSK_DIR_FORWARD) { - x = self->right; + x = self->interval.right; } else { - x = self->left; + x = self->interval.left; } while (out >= 0 && out < num_edges && out_breakpoints[out_order[out]] == x) { tsk_bug_assert(out < num_edges); @@ -4225,25 +4224,29 @@ tsk_tree_advance(tsk_tree_t *self, int direction, const double *restrict out_bre self->direction = direction; self->index = self->index + direction; if (direction == TSK_DIR_FORWARD) { - self->left = x; - self->right = sequence_length; + self->interval.left = x; + self->interval.right = sequence_length; if (out >= 0 && out < num_edges) { - self->right = TSK_MIN(self->right, out_breakpoints[out_order[out]]); + self->interval.right + = TSK_MIN(self->interval.right, out_breakpoints[out_order[out]]); } if (in >= 0 && in < num_edges) { - self->right = TSK_MIN(self->right, in_breakpoints[in_order[in]]); + self->interval.right + = TSK_MIN(self->interval.right, in_breakpoints[in_order[in]]); } } else { - self->right = x; - self->left = 0; + self->interval.right = x; + self->interval.left = 0; if (out >= 0 && out < num_edges) { - self->left = TSK_MAX(self->left, out_breakpoints[out_order[out]]); + self->interval.left + = TSK_MAX(self->interval.left, out_breakpoints[out_order[out]]); } if (in >= 0 && in < num_edges) { - self->left = TSK_MAX(self->left, in_breakpoints[in_order[in]]); + self->interval.left + = TSK_MAX(self->interval.left, in_breakpoints[in_order[in]]); } } - tsk_bug_assert(self->left < self->right); + tsk_bug_assert(self->interval.left < self->interval.right); *out_index = out; *in_index = in; if (tables->sites.num_rows > 0) { @@ -4260,9 +4263,9 @@ tsk_tree_first(tsk_tree_t *self) int ret = 1; tsk_table_collection_t *tables = self->tree_sequence->tables; - self->left = 0; + self->interval.left = 0; self->index = 0; - self->right = tables->sequence_length; + self->interval.right = tables->sequence_length; self->sites = self->tree_sequence->tree_sites[0]; self->sites_length = self->tree_sequence->tree_sites_length[0]; @@ -4279,7 +4282,7 @@ tsk_tree_first(tsk_tree_t *self) self->left_index = 0; self->right_index = 0; self->direction = TSK_DIR_FORWARD; - self->right = 0; + self->interval.right = 0; ret = tsk_tree_advance(self, TSK_DIR_FORWARD, tables->edges.right, tables->indexes.edge_removal_order, &self->right_index, tables->edges.left, @@ -4296,8 +4299,8 @@ tsk_tree_last(tsk_tree_t *self) const tsk_treeseq_t *ts = self->tree_sequence; const tsk_table_collection_t *tables = ts->tables; - self->left = 0; - self->right = tables->sequence_length; + self->interval.left = 0; + self->interval.right = tables->sequence_length; self->index = 0; self->sites = ts->tree_sites[0]; self->sites_length = ts->tree_sites_length[0]; @@ -4315,8 +4318,8 @@ tsk_tree_last(tsk_tree_t *self) self->left_index = (tsk_id_t) tables->edges.num_rows - 1; self->right_index = (tsk_id_t) tables->edges.num_rows - 1; self->direction = TSK_DIR_REVERSE; - self->left = tables->sequence_length; - self->right = 0; + self->interval.left = tables->sequence_length; + self->interval.right = 0; ret = tsk_tree_advance(self, TSK_DIR_REVERSE, tables->edges.left, tables->indexes.edge_insertion_order, &self->left_index, tables->edges.right, @@ -4367,7 +4370,7 @@ tsk_tree_prev(tsk_tree_t *self) static inline bool tsk_tree_position_in_interval(const tsk_tree_t *self, double x) { - return self->left <= x && x < self->right; + return self->interval.left <= x && x < self->interval.right; } int TSK_WARN_UNUSED @@ -4375,8 +4378,8 @@ tsk_tree_seek(tsk_tree_t *self, double x, tsk_flags_t TSK_UNUSED(options)) { int ret = 0; const double L = tsk_treeseq_get_sequence_length(self->tree_sequence); - const double t_l = self->left; - const double t_r = self->right; + const double t_l = self->interval.left; + const double t_r = self->interval.right; double distance_left, distance_right; if (x < 0 || x >= L) { @@ -4427,8 +4430,8 @@ tsk_tree_clear(tsk_tree_t *self) const bool sample_lists = !!(self->options & TSK_SAMPLE_LISTS); const tsk_flags_t *flags = self->tree_sequence->tables->nodes.flags; - self->left = 0; - self->right = 0; + self->interval.left = 0; + self->interval.right = 0; self->num_edges = 0; self->index = -1; /* TODO we should profile this method to see if just doing a single loop over diff --git a/c/tskit/trees.h b/c/tskit/trees.h index 9981c07b27..62eb70a62d 100644 --- a/c/tskit/trees.h +++ b/c/tskit/trees.h @@ -160,15 +160,19 @@ typedef struct { the tree's genomic interval. */ tsk_size_t num_edges; - + /** + @brief Left and right coordinates of the genomic interval that this + tree covers. The left coordinate is inclusive and the right coordinate + exclusive. + */ + struct { + double left; + double right; + } interval; tsk_size_t num_nodes; tsk_flags_t options; tsk_size_t root_threshold; const tsk_id_t *samples; - /* TODO before documenting this should be change to interval. */ - /* Left and right physical coordinates of the tree */ - double left; - double right; tsk_id_t index; /* These are involved in the optional sample tracking; num_samples counts * all samples below a give node, and num_tracked_samples counts those diff --git a/python/_tskitmodule.c b/python/_tskitmodule.c index 9daf12fe27..199d91c981 100644 --- a/python/_tskitmodule.c +++ b/python/_tskitmodule.c @@ -9550,7 +9550,7 @@ Tree_get_left(Tree *self) if (Tree_check_state(self) != 0) { goto out; } - ret = Py_BuildValue("d", self->tree->left); + ret = Py_BuildValue("d", self->tree->interval.left); out: return ret; } @@ -9563,7 +9563,7 @@ Tree_get_right(Tree *self) if (Tree_check_state(self) != 0) { goto out; } - ret = Py_BuildValue("d", self->tree->right); + ret = Py_BuildValue("d", self->tree->interval.right); out: return ret; }