-
Notifications
You must be signed in to change notification settings - Fork 89
Update asv #2437
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 asv #2437
Conversation
c5e03c1 to
781aaaf
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2437 +/- ##
=======================================
Coverage 91.22% 91.22%
=======================================
Files 20 20
Lines 11847 11847
Branches 2300 2300
=======================================
Hits 10807 10807
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:
|
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.
Spotted some issues.
I was also hoping that we could do basically all of the benchmarks with an object-oriented approach, where we subclass like
class Hudson(LargeSimulationBenchmark):
def _large_sample_size_params(self):
return {"sample_size": 10**6, ...}
def _run_large_sample_size(self):
self.run(self._large_sample_size_params())
def run(self, params):
return msprime.sim_ancestry(**params)
class HudsonOverRoot(Hudson)
def run(self, params):
return msprime.sim_ancestry(**{stop_at_local_mrca=False, **params})
That way we'll get decent coverage on the performance implications (which are not obvious to me - it's a deep change).
|
Very neat suggestion. I just took the liberty to propose a small structural change. Let me know what do you think. |
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.
LGTM, just needs a squash
Fix issues class based proposed modification updated commit hashes
aaaa6f9 to
508e808
Compare
|
Ready now |
Updated the code to work with msprime 1.x syntax (sim_ancestry instead of simulate), assuming that benchmarking against msprime 0.x is not longer necessary.
Included time and memory tests for stop_at_local_mrca=False
updated the hashes list in the file, and included a line to run from a hashfile instead of just going all commits in order for an hour