Skip to content

Conversation

@hossam26644
Copy link
Contributor

@hossam26644 hossam26644 commented Jan 26, 2026

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

@hossam26644 hossam26644 changed the title Simplify the OOA verification test for the smck model Simplify the OOA verification test for the smck model fixes #2462 Jan 26, 2026
@hossam26644 hossam26644 changed the title Simplify the OOA verification test for the smck model fixes #2462 Simplify the OOA verification test for the smck model fix 2462 Jan 26, 2026
@codecov
Copy link

codecov bot commented Jan 26, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.21%. Comparing base (8b6acb5) to head (c536518).
⚠️ Report is 9 commits behind head on main.

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           
Flag Coverage Δ
C 91.21% <ø> (ø)
python 98.63% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@hossam26644
Copy link
Contributor Author

I added the fix for #2463 here so that you won't have to deal with merge conflict

@hossam26644 hossam26644 changed the title Simplify the OOA verification test for the smck model fix 2462 Simplify the smck OOA verification and refactor smck verifications fix 2462, 2463 Jan 26, 2026
Copy link
Member

@jeromekelleher jeromekelleher left a 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
Copy link
Member

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is how it looks if dropped to one sample and do 20 replicates instead of 10 (takes ~30 seconds). We can also reduce the recombination rate by one order of magnitude and keep the large number of samples.

It is slow because the sampling algorithm takes a lot of time.

qq

@hossam26644
Copy link
Contributor Author

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

@hossam26644
Copy link
Contributor Author

hossam26644 commented Jan 27, 2026

For the test_smc_k_oldest_time:
num_loci = np.linspace(100, 10**4, 10).astype(int)mean
num_loci = np.linspace(100, 10**5, 10).astype(int)mean (3)

@hossam26644
Copy link
Contributor Author

Variance plots for test_smc_k_num_trees:
L = np.linspace(100, 10**4, 10).astype(int)
var
L = np.linspace(100, 10**5, 10).astype(int)
var (2)

@jeromekelleher
Copy link
Member

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
@hossam26644 hossam26644 force-pushed the fix/test-ooa-takes-too-long-2462 branch from 25c5f66 to c536518 Compare January 27, 2026 14:39
@hossam26644
Copy link
Contributor Author

check now @jeromekelleher

@jeromekelleher jeromekelleher added this pull request to the merge queue Jan 27, 2026
Merged via the queue into tskit-dev:main with commit 15a702a Jan 27, 2026
14 checks passed
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 this pull request may close these issues.

test_out_of_africa_migration_model takes too long to run

2 participants