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

Add (invalid) test cases of proportions == 0.0 #167

Merged
merged 3 commits into from
Nov 19, 2024

Conversation

molpopgen
Copy link
Contributor

@molpopgen molpopgen commented Nov 18, 2024

@grahamgower
Copy link
Member

Thanks for getting this rolling... I have a couple of additional test cases (for the defaults section), and the corresponding fixes for the reference implementation. I just ran out of time to commit and pull request yesterday. I can just add them on top of your PR, if you don't mind?

@molpopgen
Copy link
Contributor Author

@grahamgower -- go for it!

@molpopgen
Copy link
Contributor Author

@grahamgower -- I just rebased this onto main so that CI works.

This also uses the terms 'proportion' and 'rate' to match the
json schema, rather than using the term 'fraction'.
@grahamgower
Copy link
Member

@molpopgen, could you please run your eye over the changes I added here? If you're ok with it, I think it can be merged.

@molpopgen
Copy link
Contributor Author

LGTM but not sure why some changes concern selfing/cloning rates?

@grahamgower
Copy link
Member

In the schema definitions, we have (a) proportion for ancestry proportions and pulse proportions, where 0 < proportion <= 1, and (b) we have rate for migration rates, selfing/cloning, where 0 <= rate <= 1. The code in the reference implementation instead used the term 'fraction' and this was used for both proportions and rates, so I removed 'fraction' in favour of using the same terms and definitions as the schema.

@molpopgen
Copy link
Contributor Author

Seems like this one is ready to go!

@grahamgower grahamgower merged commit 0f5fa52 into main Nov 19, 2024
4 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.

2 participants