-
Notifications
You must be signed in to change notification settings - Fork 91
Simplify the smck OOA verification and refactor smck verifications fix 2462, 2463 #2468
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
Simplify the smck OOA verification and refactor smck verifications fix 2462, 2463 #2468
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2468 +/- ##
=======================================
Coverage 91.21% 91.21%
=======================================
Files 20 20
Lines 11845 11845
Branches 2300 2300
=======================================
Hits 10805 10805
Misses 569 569
Partials 471 471
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
I added the fix for #2463 here so that you won't have to deal with merge conflict |
jeromekelleher
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's still quite long. I'd rather do a smaller simulation with more replicates here. Why does it need to take so much time?
verification.py
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can drop this to 1 diploid sample each, see if that brings the time down.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
since you mentioned it, test_smc_k_num_trees and test_smc_k_oldest_time take long as well. But I think the plots are more informative with the long time vs when I simplify the parameters. I will followup with how the figures look simplified and non simplified |
|
OK looks good. But the basic point is that more replicates of a smaller simulation is more useful here, so let's optimise for that. I know some of the other tests are a bit slow but they need to be because you need lots of replicates to get variance. This doesn't need to have lots of replicates, or to be a big simulation. |
refactor smck verification test (remove usless superclass) simplify ooa
25c5f66 to
c536518
Compare
|
check now @jeromekelleher |





fixes #2462
Now it takes: real 1m51.020s on my PC.
@jeromekelleher let me know if the time is still too long