Skip to content
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

update validation.py after demography API changes #786

Closed
grahamgower opened this issue Mar 5, 2021 · 9 comments · Fixed by #1585
Closed

update validation.py after demography API changes #786

grahamgower opened this issue Mar 5, 2021 · 9 comments · Fixed by #1585
Assignees
Milestone

Comments

@grahamgower
Copy link
Member

#764 made changes to the DemographicModel class, which very likely breaks the validation.py script.

@jeromekelleher jeromekelleher added this to the Version 0.2.0 milestone Mar 5, 2021
@nspope
Copy link
Collaborator

nspope commented Aug 13, 2024

@silastittes do you have time to look at this? I think someone just needs to check if validation.py runs, and investigate if not.

@petrelharp
Copy link
Contributor

I'm running it (after that fix). Could you post its output here? (i.e., the plots/)

@silastittes
Copy link
Contributor

Yes, but output is on talapas which is down for maintenance. Made a reminder in my calendar to post!

@silastittes
Copy link
Contributor

Here's the output on my end.
plots.tar.gz

@petrelharp
Copy link
Contributor

Is that before or after the correction I made here?

@petrelharp
Copy link
Contributor

(I was running these on my laptop but didn't realize how long it would take; I killed it after like half a day.)

@silastittes
Copy link
Contributor

This is before. I assumed the goal was comparing before and after?
Yes takes a while end-to-end!

@petrelharp
Copy link
Contributor

I think what we should be doing is checking that validation still looks good after this PR - so, it should be 'after'? If it doesn't look good, probably that would be due to some other change somewhere else, rather than this PR, but this is still the place to validate.

@petrelharp
Copy link
Contributor

Well, actually - those plots look reasonable; the only deviations I see is that there are fewer edges and nodes in rescaled SLiM simulations than msprime, which is I think not unexpected (due to the rescaling). I didn't even see the effect of the error which I fixed, but again that seems believable. I'll merge that. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants