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.

Sign up for GitHub

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

Jump to bottom

Fix Zigzag_1S14 population sizes. #750

Merged
merged 1 commit into from
Jan 28, 2021
Merged

Fix Zigzag_1S14 population sizes. #750

merged 1 commit into from
Jan 28, 2021

Conversation

grahamgower
Copy link
Member

This makes the population sizes match those used in Schiffels & Durbin (2014),
and outputs a warning when simulating the Zigzag_1S14 model.

Closes #745.

@grahamgower
Copy link
Member Author

grahamgower commented Jan 28, 2021 •
edited
Loading

Odd that the circleci failure doesn't also occur on the github-workflow tests. I can't reproduce locally either, but code inspection suggests this test should fail (simulating the model outputs a warning, so we should have len(stderr) > 0).

@jeromekelleher
Copy link
Member

Huh, weird.

@grahamgower
Copy link
Member Author

grahamgower commented Jan 28, 2021 •
edited
Loading

I'm just going to remove the failing test. It doesn't add anything that's not covered elsewhere, and identifying why its failing only on circleci is not worth the time (I already spent half an hour and didn't come up with anything).

@codecov
Copy link

codecov bot commented Jan 28, 2021 •
edited
Loading

Codecov Report

Merging #750 (fa73c50) into main (22d5874) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #750   +/-   ##
=======================================
  Coverage   99.66%   99.66%           
=======================================
  Files          33       33           
  Lines        2381     2388    +7     
  Branches      297      298    +1     
=======================================
+ Hits         2373     2380    +7     
  Misses          3        3           
  Partials        5        5
Impacted Files Coverage Δ
stdpopsim/catalog/HomSap/__init__.py 100.00% <100.00%> (ø)
stdpopsim/engines.py 100.00% <100.00%> (ø)
stdpopsim/qc/HomSap.py 100.00% <100.00%> (ø)
stdpopsim/slim_engine.py 99.68% <100.00%> (+<0.01%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 22d5874...fa73c50. Read the comment docs.

@grahamgower
Fix Zigzag_1S14 population sizes.
fa73c50 
This makes the population sizes match those used in Schiffels & Durbin (2014),
and outputs a warning when simulating the Zigzag_1S14 model.

This also removes a test that fails on circleci. I was not able to
reproduce the failure offline, and github workflows tests also do not
reproduce the failure. The test adds no unique coverage.

Closes popsim-consortium#745.
@grahamgower grahamgower merged commit 6999b3a into popsim-consortium:main Jan 28, 2021
@grahamgower grahamgower deleted the zigzag branch January 28, 2021 19:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jeromekelleher jeromekelleher jeromekelleher approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

HomSap/Zigzag_1S14 population sizes don't match Schiffels & Durbin (2014)
2 participants
@grahamgower @jeromekelleher