-
Notifications
You must be signed in to change notification settings - Fork 71
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support migrations in simplify #20
Comments
#567 changed the behaviour to raise an error when attempting to simplify a set of tables with migrations. |
When tackling this, add a check to the Another aspect of migration simplification is that we should take references to populations from migrations into account too. |
Ping @Chris1221 who was interested in taking a look at this. I'm not sure how complex an issue this would be for someone unfamiliar with the codebase, but Jerome's comment at the top gives me hope that it's not too bad |
Not the ideal place to start, as simplify is pretty hairy and multi-layered. But, would be a great way to learn the codebase and how the top-to-bottom development process works. |
Perfect, thank you @hyanwong! Yes, probably won't start here, but I'll keep it on my radar. This feature would be particularly useful for the |
It's not so bad @Chris1221 --- I'd be happy to help you get up to speed. It'd be great to get smcsmc converted to tskit! |
Yea I think so too. I was talking to @hyanwong a little about it, but we've implemented a very hacky conversion to the basic tables in text format -- it needs quite a bit of refining but the major issue was that we treat migration events as nodes, which is different than |
Amen brother! I think it'd be well worth while spending a few days with us at the BDI then, I'm sure we can iron out the details between us. |
That sounds great! I'm going home for Christmas soon, so realistically I don't think I'll have enough time in the next week or so, but we should try to find some time early in the new year. It's pretty high on my to-do list for |
I'm going to take a look at this in the next few days. If anyone else has been working on it, let me know! |
Nope, this is all yours @Chris1221! I'm happy to have a quick chat to get you up to speed - do send me an email/slack if you have any questions. |
This seems to have dropped off the radar. It would be nice to have for one of the tuts. Any takers to finish it off? @Chris1221 I assume you never found time to do it? |
So sorry @hyanwong, I started working on it but time in my PhD is running out so I never got around to finishing it. My apologies. :( |
I've added this to C 1.0 as we should think if it has any effect on the simplify API. (I don't think so but worth some more thought) |
Moving to C upcoming as doesn't have an impact on the public API. |
Just to record that we should remember to update the doc strings for the |
Simplify current ignores migration records. We need to update simplify so that migrations are also processed. This should be simple to do; we just use the node ID map to translate the node IDs in the old table into new node IDs. If the old node ID maps to -1, we leave the migration out.
The text was updated successfully, but these errors were encountered: