Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 32 additions & 1 deletion c/tests/test_tables.c
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ test_table_collection_simplify_errors(void)
int ret;
tsk_table_collection_t tables;
tsk_id_t samples[] = { 0, 1 };

const char *individuals = "1 0.25 -2\n";
ret = tsk_table_collection_init(&tables, 0);
CU_ASSERT_EQUAL_FATAL(ret, 0);
tables.sequence_length = 1;
Expand All @@ -296,6 +296,14 @@ test_table_collection_simplify_errors(void)
tables.sites.position[0] = 1.5;
ret = tsk_table_collection_simplify(&tables, samples, 0, 0, NULL);
CU_ASSERT_EQUAL_FATAL(ret, TSK_ERR_BAD_SITE_POSITION);
tsk_site_table_truncate(&tables.sites, 0);
tables.sites.position[0] = 0;

/* Individual out of bounds */
parse_individuals(individuals, &tables.individuals);
CU_ASSERT_EQUAL_FATAL(tables.individuals.num_rows, 1);
ret = tsk_table_collection_simplify(&tables, samples, 0, 0, NULL);
CU_ASSERT_EQUAL_FATAL(ret, TSK_ERR_INDIVIDUAL_OUT_OF_BOUNDS);

/* TODO More tests for this: see
* https://github.com/tskit-dev/msprime/issues/517 */
Expand Down Expand Up @@ -4755,6 +4763,9 @@ test_table_collection_check_integrity_with_options(tsk_flags_t tc_options)
{
int ret;
tsk_table_collection_t tables;
const char *individuals = "1 0.25 -1\n"
"2 0.5,0.25 2\n"
"3 0.5,0.25 0\n";

ret = tsk_table_collection_init(&tables, tc_options);
CU_ASSERT_EQUAL_FATAL(ret, 0);
Expand Down Expand Up @@ -5177,6 +5188,26 @@ test_table_collection_check_integrity_with_options(tsk_flags_t tc_options)
CU_ASSERT_FATAL(ret >= 0);
ret = tsk_table_collection_check_integrity(&tables, 0);
CU_ASSERT_EQUAL_FATAL(ret, TSK_ERR_BAD_EDGE_INTERVAL);
ret = tsk_migration_table_clear(&tables.migrations);
CU_ASSERT_EQUAL_FATAL(ret, 0);

parse_individuals(individuals, &tables.individuals);
CU_ASSERT_EQUAL_FATAL(tables.individuals.num_rows, 3);
ret = tsk_table_collection_check_integrity(&tables, TSK_CHECK_INDIVIDUAL_ORDERING);
CU_ASSERT_EQUAL_FATAL(ret, TSK_ERR_UNSORTED_INDIVIDUALS);
ret = tsk_table_collection_check_integrity(&tables, 0);
CU_ASSERT_EQUAL_FATAL(ret, 0);

/* Check that an individual can't be its own parent */
tables.individuals.parents[0] = 0;
tables.individuals.parents[1] = 1;
tables.individuals.parents[2] = 2;
ret = tsk_table_collection_check_integrity(&tables, 0);
CU_ASSERT_EQUAL_FATAL(ret, TSK_ERR_INDIVIDUAL_SELF_PARENT);

tables.individuals.parents[0] = -2;
ret = tsk_table_collection_check_integrity(&tables, 0);
CU_ASSERT_EQUAL_FATAL(ret, TSK_ERR_INDIVIDUAL_OUT_OF_BOUNDS);

tsk_table_collection_free(&tables);
}
Expand Down
42 changes: 35 additions & 7 deletions c/tests/test_trees.c
Original file line number Diff line number Diff line change
Expand Up @@ -1993,6 +1993,8 @@ test_simplest_bad_individuals(void)
const char *edges = "0 1 2 0\n"
"0 1 2 1\n"
"0 1 4 3\n";
const char *individuals = "1 0.25 -1\n"
"2 0.5,0.25 0\n";
tsk_treeseq_t ts;
tsk_table_collection_t tables;
tsk_flags_t load_flags = TSK_BUILD_INDEXES;
Expand Down Expand Up @@ -2028,13 +2030,14 @@ test_simplest_bad_individuals(void)
tsk_treeseq_free(&ts);
tables.nodes.individual[0] = TSK_NULL;

/* add two individuals */
ret = tsk_individual_table_add_row(
&tables.individuals, 0, NULL, 0, NULL, 0, NULL, 0);
CU_ASSERT_EQUAL(ret, 0);
ret = tsk_individual_table_add_row(
&tables.individuals, 0, NULL, 0, NULL, 0, NULL, 0);
CU_ASSERT_EQUAL(ret, 1);
/* Add two individuals */
parse_individuals(individuals, &tables.individuals);
CU_ASSERT_EQUAL_FATAL(tables.individuals.num_rows, 2);

/* Make sure we have a good set of records */
ret = tsk_treeseq_init(&ts, &tables, load_flags);
CU_ASSERT_EQUAL_FATAL(ret, 0);
tsk_treeseq_free(&ts);

/* Bad individual ID */
tables.nodes.individual[0] = 2;
Expand All @@ -2043,6 +2046,31 @@ test_simplest_bad_individuals(void)
tsk_treeseq_free(&ts);
tables.nodes.individual[0] = TSK_NULL;

/* Bad parent ID */
tables.individuals.parents[0] = -2;
ret = tsk_treeseq_init(&ts, &tables, load_flags);
CU_ASSERT_EQUAL(ret, TSK_ERR_INDIVIDUAL_OUT_OF_BOUNDS);
tsk_treeseq_free(&ts);
tables.individuals.parents[0] = 42;
ret = tsk_treeseq_init(&ts, &tables, load_flags);
CU_ASSERT_EQUAL(ret, TSK_ERR_INDIVIDUAL_OUT_OF_BOUNDS);
tsk_treeseq_free(&ts);
tables.individuals.parents[0] = TSK_NULL;

/* Parent is self */
tables.individuals.parents[0] = 0;
ret = tsk_treeseq_init(&ts, &tables, load_flags);
CU_ASSERT_EQUAL(ret, TSK_ERR_INDIVIDUAL_SELF_PARENT);
tsk_treeseq_free(&ts);
tables.individuals.parents[0] = TSK_NULL;

/* Unsorted individuals */
tables.individuals.parents[0] = 1;
ret = tsk_treeseq_init(&ts, &tables, load_flags);
CU_ASSERT_EQUAL(ret, TSK_ERR_UNSORTED_INDIVIDUALS);
tsk_treeseq_free(&ts);
tables.individuals.parents[0] = TSK_NULL;

tsk_treeseq_free(&ts);
tsk_table_collection_free(&tables);
}
Expand Down
10 changes: 10 additions & 0 deletions c/tskit/core.c
Original file line number Diff line number Diff line change
Expand Up @@ -474,6 +474,16 @@ tsk_strerror_internal(int err)
case TSK_ERR_KEEP_UNARY_MUTUALLY_EXCLUSIVE:
ret = "You cannot specify both KEEP_UNARY and KEEP_UNARY_IN_INDIVDUALS.";
break;

/* Individual errors */
case TSK_ERR_UNSORTED_INDIVIDUALS:
ret = "Individuals must be provided in an order where children are after "
"their parent individuals";
break;

case TSK_ERR_INDIVIDUAL_SELF_PARENT:
ret = "Individuals cannot be their own parents.";
break;
}
return ret;
}
Expand Down
4 changes: 4 additions & 0 deletions c/tskit/core.h
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,10 @@ not found in the file.
/* Simplify errors */
#define TSK_ERR_KEEP_UNARY_MUTUALLY_EXCLUSIVE -1600

/* Individual errors */
#define TSK_ERR_UNSORTED_INDIVIDUALS -1700
#define TSK_ERR_INDIVIDUAL_SELF_PARENT -1701

// clang-format on

/* This bit is 0 for any errors originating from kastore */
Expand Down
44 changes: 43 additions & 1 deletion c/tskit/tables.c
Original file line number Diff line number Diff line change
Expand Up @@ -7943,6 +7943,43 @@ tsk_table_collection_check_migration_integrity(
return ret;
}

static int TSK_WARN_UNUSED
tsk_table_collection_check_individual_integrity(
const tsk_table_collection_t *self, tsk_flags_t options)
{
int ret = 0;
tsk_size_t j, k;
const tsk_individual_table_t individuals = self->individuals;
const tsk_id_t num_individuals = (tsk_id_t) individuals.num_rows;
const bool check_individual_ordering = options & TSK_CHECK_INDIVIDUAL_ORDERING;

for (j = 0; j < (tsk_size_t) num_individuals; j++) {
for (k = individuals.parents_offset[j]; k < individuals.parents_offset[j + 1];
k++) {
/* Check parent references are valid */
if (individuals.parents[k] != TSK_NULL
&& (individuals.parents[k] < 0
|| individuals.parents[k] >= num_individuals)) {
ret = TSK_ERR_INDIVIDUAL_OUT_OF_BOUNDS;
goto out;
}
/* Check no-one is their own parent */
if (individuals.parents[k] == (tsk_id_t) j) {
ret = TSK_ERR_INDIVIDUAL_SELF_PARENT;
goto out;
}
/* Check parents are ordered */
if (check_individual_ordering && individuals.parents[k] != TSK_NULL
&& individuals.parents[k] >= (tsk_id_t) j) {
ret = TSK_ERR_UNSORTED_INDIVIDUALS;
goto out;
}
}
}
out:
return ret;
}

static tsk_id_t TSK_WARN_UNUSED
tsk_table_collection_check_tree_integrity(const tsk_table_collection_t *self)
{
Expand Down Expand Up @@ -8074,7 +8111,7 @@ tsk_table_collection_check_integrity(
/* Checking the trees implies all the other checks */
options |= TSK_CHECK_EDGE_ORDERING | TSK_CHECK_SITE_ORDERING
| TSK_CHECK_SITE_DUPLICATES | TSK_CHECK_MUTATION_ORDERING
| TSK_CHECK_INDEXES;
| TSK_CHECK_INDEXES | TSK_CHECK_INDIVIDUAL_ORDERING;
}

if (self->sequence_length <= 0) {
Expand Down Expand Up @@ -8105,6 +8142,11 @@ tsk_table_collection_check_integrity(
if (ret != 0) {
goto out;
}
ret = tsk_table_collection_check_individual_integrity(self, options);
if (ret != 0) {
goto out;
}

if (options & TSK_CHECK_INDEXES) {
ret = tsk_table_collection_check_index_integrity(self);
if (ret != 0) {
Expand Down
3 changes: 3 additions & 0 deletions c/tskit/tables.h
Original file line number Diff line number Diff line change
Expand Up @@ -720,6 +720,7 @@ typedef struct {
#define TSK_CHECK_MUTATION_ORDERING (1 << 3)
#define TSK_CHECK_INDEXES (1 << 4)
#define TSK_CHECK_TREES (1 << 5)
#define TSK_CHECK_INDIVIDUAL_ORDERING (1 << 6)

/* Leave room for more positive check flags */
#define TSK_NO_CHECK_POPULATION_REFS (1 << 10)
Expand Down Expand Up @@ -2999,6 +3000,8 @@ TSK_CHECK_MUTATION_ORDERING
Check contraints on the ordering of mutations. Any non-null
mutation parents and known times are checked for ordering
constraints.
TSK_CHECK_INDIVIDUAL_ORDERING
Check individual parents are before children, where specified.
TSK_CHECK_INDEXES
Check that the table indexes exist, and contain valid edge
references.
Expand Down
6 changes: 3 additions & 3 deletions docs/data-model.rst
Original file line number Diff line number Diff line change
Expand Up @@ -540,10 +540,10 @@ Individual requirements
-----------------------

Individuals are a basic type in a tree sequence and are not defined with
respect to any other tables. Therefore, there are no requirements on
individuals.
respect to any other tables. Individuals can have a reference to their parent
individuals, if present these references must be valid or null (-1).

There are no requirements regarding the ordering of individuals.
Individuals must be sorted such that parents are before children.
Sorting a set of tables using :meth:`TableCollection.sort` has
no effect on the individuals.

Expand Down
2 changes: 1 addition & 1 deletion python/CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
(:user:`hyanwong`,:issue:`1155`, :pr:`1182`).

- Add ``parents`` column to the individual table to allow recording of pedigrees
(:user:`ivan-krukov`, :user:`benjeffery`, :issue:`852`, :pr:`1125`, :pr:`866`, :pr:`1153`, :pr:`1177`).
(:user:`ivan-krukov`, :user:`benjeffery`, :issue:`852`, :pr:`1125`, :pr:`866`, :pr:`1153`, :pr:`1177` :pr:`1192`).

- Added ``Tree.generate_random_binary`` static method to create random
binary trees (:user:`hyanwong`, :user:`jeromekelleher`, :pr:`1037`).
Expand Down
2 changes: 1 addition & 1 deletion python/lwt_interface/dict_encoding_testlib.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ def full_ts():
# TODO replace this with properly linked up individuals using sim_ancestry
# once 1.0 is released.
for j in range(n):
tables.individuals.add_row(flags=j, location=(j, j), parents=(j, j))
tables.individuals.add_row(flags=j, location=(j, j), parents=(j - 1, j - 1))

for name, table in tables.name_map.items():
if name != "provenances":
Expand Down
2 changes: 1 addition & 1 deletion python/tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ def ts_fixture():
# TODO replace this with properly linked up individuals using sim_ancestry
# once 1.0 is released.
for j in range(n):
tables.individuals.add_row(flags=j, location=(j, j), parents=(j, j))
tables.individuals.add_row(flags=j, location=(j, j), parents=(j - 1, j - 1))

for name, table in tables.name_map.items():
if name != "provenances":
Expand Down
23 changes: 23 additions & 0 deletions python/tests/test_tables.py
Original file line number Diff line number Diff line change
Expand Up @@ -2361,6 +2361,29 @@ def test_samples_interface(self):
with pytest.raises(OverflowError):
tables.simplify(samples=np.array([0, bad_node]))

def test_bad_individuals(self, simple_ts_fixture):
tables = simple_ts_fixture.dump_tables()
tables.individuals.clear()
tables.individuals.add_row(parents=[-2])
with pytest.raises(tskit.LibraryError, match="Individual out of bounds"):
tables.simplify()
tables.individuals.clear()
tables.individuals.add_row(parents=[0])
with pytest.raises(
tskit.LibraryError, match="Individuals cannot be their own parents"
):
tables.simplify()

def test_unsorted_individuals_ok(self, simple_ts_fixture):
tables = simple_ts_fixture.dump_tables()
tables.individuals.clear()
tables.individuals.clear()
tables.individuals.add_row(parents=[1])
tables.individuals.add_row(parents=[-1])
# we really just want to check that no error is thrown here
tables.simplify()
assert tables.individuals.num_rows == 0


class TestTableCollection:
"""
Expand Down