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

N function in pedigree sims #2322

Merged
merged 1 commit into from
Oct 8, 2024
Merged

Conversation

hannesbecher
Copy link
Contributor

When simulating pedigrees, one needs to set a population size N. This PR adds functionality for N to be a function of the generation (back in time). This works for forward and backward pedigree simulations.

Copy link

codecov bot commented Oct 3, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.93%. Comparing base (e2932a2) to head (97f7f91).
Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2322   +/-   ##
=======================================
  Coverage   90.93%   90.93%           
=======================================
  Files          20       20           
  Lines       12014    12018    +4     
  Branches     2438     2439    +1     
=======================================
+ Hits        10925    10929    +4     
  Misses        602      602           
  Partials      487      487           
Flag Coverage Δ
C 90.93% <100.00%> (+<0.01%) ⬆️
python 98.70% <100.00%> (+<0.01%) ⬆️

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.

@jeromekelleher
Copy link
Member

LGTM - just needs some light test coverage I guess.

Some ideas:

  • passing in population_size=lamba _: N should be identical to population_size=N
  • passing in population_size=lamba t: 100 t < 10 else 1 should cause an immediate bottleneck at time 10

@hannesbecher
Copy link
Contributor Author

LGTM - just needs some light test coverage I guess.

Some ideas:

* passing in `population_size=lamba _: N` should be identical to `population_size=N`

* passing in `population_size=lamba t: 100 t < 10 else 1` should cause an immediate bottleneck at time 10

I've tried. Do the added tests make sense?

@hannesbecher hannesbecher marked this pull request as ready for review October 3, 2024 14:32
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.

Nice, just need to reformat the docstring style.

"""
Simulates a pedigree

Arguments:
Copy link
Member

Choose a reason for hiding this comment

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

Can you reformat this using sphinx :param: sections like the rest of the code please?

@jeromekelleher
Copy link
Member

Great! Can you squash down to one commit please? (See dev docs for guidance)

@hannesbecher
Copy link
Contributor Author

NB, squashing is mentioned only in the tskit dev docs, I think.

@jeromekelleher jeromekelleher merged commit feed38d into tskit-dev:main Oct 8, 2024
14 checks passed
@hannesbecher hannesbecher deleted the pedSimDev branch October 8, 2024 15:18
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.

2 participants