test case where one ts adds an extra pop

[petrelharp]

that is not there in the first one, but existing pops should be identified

[petrelharp]

Oh, but new nodes want to be in a new population - should document how this happens?

[mufernando]

Right now I am creating a new population for each pop that appears in the nodes to be added. See here.

Also, the function will return a map from pop in ts2 to pop in the grafted ts.

[mufernando]

actually the n.population = pop_map2new[n.population] should not be in an else statement.

[mufernando]

Ok, added sth to the doc and fixed the above issue. All nodes that are new to ts1 receive a new population ID and a map (ts2 to grafted ts) is returned.

See here.

[petrelharp]

Looks good! Is this behavior being tested?

[mufernando]

No! Not sure how to test it?

[petrelharp]

Well, if you write a test that adds just one new node to an existing tree sequence, you can check that that node has a new population?

Or more generally, for any test case where you know which nodes from ts2 should be added, you can check that all those nodes are given new populations not previously present using the output node map?

[mufernando]

I think I got this: here

[petrelharp]

Hm, I think we can be more precise. How about something like this:

    def verify_node_populations(self, ts1, ts2, node_map21):
        tsg, (node_map2new, pop_map2new, ind_map2new) = graft(ts1, ts2, node_map21)
        # check that ts1 pops remain unchanged
        for j, p in enumerate(ts1.populations()):
            self.assertEqual(p, tsg.population(j))
        # check that each ts2 pop maps to a unique tsg pop
        self.assertEqual(len(pop_map2new.keys()), len(set(pop_map2new.values())))
        # check that ts2 pops are unchanged, and that newly added nodes have a new population
        for n2 in range(ts2.num_nodes):
            p2 =ts2.node(n2).population
            ng = node_map2new[n2]
            pg = tsg.node(ng).population
            self.assertEqual(ts2.population(p2), tsg.population(pg))
            if n2 not in node_map21:
                self.assertGreater(pg, ts1.num_populations)

[mufernando]

thanks! implemented here

[petrelharp]

looks good. I would vote for not passing in tsg etcetera so that it makes the test more clear - you can tell what it does just by looking at it, and don't have to worry about getting the right pop_map2new, etcetera. And, this is tests, so we don't have to worry about efficiency. But, no big deal.

[mufernando]

Agreed! d41d73379bf6647ffe4d42ae61749001d3119f38