Transition to msprime 1.0 demography model #764
Conversation
|
Yeah, that sounds like a good transition plan. Of course, the devil will be in the details. 😈 |
jeromekelleher
force-pushed
the
msprime-1-demog
branch
3 times, most recently
from
49af25d
to
eb95ffe
Compare
|
@grahamgower, it'd be good to get your eyes on this at this point. I've got stuff mostly working - the CLI is working with the msprime engine, and I think we can work through all the rest of the test failures easily enough. The basic idea is to basically get rid of all the code we have for modelling populations/demography etc and to push this (first) out to msprime and then later to Demes. If people have strayed off the simple "run a simulation" example code and written stuff that depends on the internal structure of the stdpopsim objects, then we'll break their code. But, mostly I think the update shouldn't affect people. The next step here would be to make the minimal changes to get the QC and production models agreeing, via What do you think? |
|
There's quite a lot of breakage here in the |
I guess we can try to keep compatibility with this, but I wonder if we should. Most of the API documentation there was "premature documentation", it should never have been exposed as public in the first place. I think we only put the API documentation in here for models etc for stdpopsim developers - I certainly didn't have a long-term stable API in mind there. In general, we're going to be pretty hidebound if we try to keep full compatibility with all these details, and end up doing an awful lot of extra work, probably for nothing. Do you think anyone is actually using the DemographicModel API? |
That's hard to guess at. Maybe it's worth asking on slack? |
jeromekelleher
force-pushed
the
msprime-1-demog
branch
from
1183dd6
to
a69a661
Compare
|
@petrelharp @andrewkern This is worth taking a look at now. There's still some details to be worked out, but the basic idea is that we get rid of as much of our local code for defining demographic models as we can, and lean on the msprime tools. I've also updated to use msprime 1.0 APIs for the msprime engine, and updated from RecombinationMap to RateMap everywhere. What do we think of the basic direction? I think we just have to view the old implementation of DemographicModel as an internal implementation detail that wasn't documented/intended for end users. We'll never get through the transition to demes otherwise. |
Codecov Report
@@ Coverage Diff @@
## main #764 +/- ##
==========================================
- Coverage 99.54% 99.48% -0.07%
==========================================
Files 36 36
Lines 2429 2337 -92
Branches 303 279 -24
==========================================
- Hits 2418 2325 -93
- Misses 5 6 +1
Partials 6 6
Continue to review full report at Codecov.
|
|
This looks good! I'm not sure I have enough of the big picture to be more specific, though. |
jeromekelleher
force-pushed
the
msprime-1-demog
branch
from
a69a661
to
73ca6d8
Compare
|
This is ready for review @grahamgower. I think most things went fairly smoothly - the plan for incrementally modernising the catalog should work, I think. The only thing I'm not sure about is non-sampling pops, which I've commented on elsewhere. There'll be a batch of stuff to do after this, which I guess is best handled by opening issues. If you see things that are outstanding (like, say, updating the recapitation bit of the SLiM engine) which aren't quick fixes, can you open some issues to track please? Wrt to the API breakage for the DemographicModel class, I suggest we (a) undocument everything except Developers adding new models should only need to use msprime APIs for now, and later Demes. We should sort out #714 before the next release. |
This is a transitional update, which gets most features working with msprime 1.0, without starting the process of modernising the catalog models. Also updates the DemographicModel class to remove instance variables that were tied to the old representation and their documentation. Closes popsim-consortium#748 Closes popsim-consortium#740 Closes popsim-consortium#535
jeromekelleher
force-pushed
the
msprime-1-demog
branch
from
73ca6d8
to
5a1ddc9
Compare
|
OK, I think this is ready to go. @grahamgower, I'll let you do the merging honours! |
This is an initial pass at converting the codebase to use the msprime 1.0 Demography model, rather than the msprime 0.x model as we currently do.
This will be a messy change, but I think it's something we can do incrementally, which will also move us closer to demes (which will be the future basis for demography).
The first pass here is to just remove the model comparison code we have here, which is using internal msprime APIs, and use msprime's APIs instead.
My proposal for the transition would be the following:
population_configurations,demographic_eventsandmigration_matrixreplace with ademographyobject. (May get rid of thepopulationslist as well, since this is redundant, but not in the first pass)Demography.from_old_styleon all the places we create a model object.This much should work, and we should be able to merge.
Once that's working, we can then start tweaking the model definitions to make them more idiomatic/more demes-like. Ultimately, I'd like to make some demes-like Demographys for the stuff in the catalog, where we use population splits etc rather than mass migrations. We then compare them with the qc models using the
is_equivalent()method, which is able to take into account populations being reused, via a mapping of equivalent populations epoch-by-epoch. That way, we should be able to keep the QC models as they are, but incrementally modernise the catalog models.Upgrading from the msprime demes-like Demography model should be straightforward enough then at a later date.
What do you think @grahamgower?