Skip to content

Conversation

@jdebacker
Copy link
Member

This PR updates demographics.py to use the UN data to determine the population distribution in the period before the model starts. The module had been using the period 1 population distribution, fertility, mortality, and immigration rates to work backwards to solve for this. But in some countries (e.g. Ethiopia) there are large swings in these from year to year and so the inferred population was not consistent with the actual population.

In addition, a change is made to a client.gather call since dask has removed the timeout kwarg.

Addresses Issue #1070.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR addresses Issue #1070 by updating the demographics module to use actual UN historical data for determining pre-time path population distribution instead of solving backwards from period 1 rates. This change improves accuracy in countries like Ethiopia that experience large year-to-year swings in demographic parameters. Additionally, the PR removes the deprecated timeout parameter from a client.gather() call to maintain compatibility with updated versions of dask.

  • Replaces backward-solving logic for pre-period population with direct use of UN historical data
  • Adds validation and warning logic to verify consistency between UN pre-period data and period 0 population
  • Updates dask client call to remove deprecated timeout parameter

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.

File Description
ogcore/demographics.py Comments out the old backward-solving implementation and replaces it with direct use of UN historical data for pre-period population, adds verification logic with warning for large inconsistencies
ogcore/SS.py Removes deprecated timeout parameter from client.gather() call to maintain compatibility with updated dask versions

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@codecov-commenter
Copy link

codecov-commenter commented Nov 14, 2025

Codecov Report

❌ Patch coverage is 85.71429% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 72.44%. Comparing base (df7b128) to head (bfb15a7).

Files with missing lines Patch % Lines
ogcore/demographics.py 80.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1071      +/-   ##
==========================================
- Coverage   72.70%   72.44%   -0.27%     
==========================================
  Files          21       21              
  Lines        5111     5098      -13     
==========================================
- Hits         3716     3693      -23     
- Misses       1395     1405      +10     
Flag Coverage Δ
unittests 72.44% <85.71%> (-0.27%) ⬇️

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

Files with missing lines Coverage Δ
ogcore/SS.py 71.45% <100.00%> (-1.97%) ⬇️
ogcore/__init__.py 100.00% <100.00%> (ø)
ogcore/demographics.py 53.63% <80.00%> (-2.05%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@rickecon
Copy link
Member

rickecon commented Nov 14, 2025

This looks good to me. Thanks for running this down. And I look forward to working on a more robust solution to Issue #1070 in the future.

@rickecon rickecon merged commit b5a3ebd into PSLmodels:master Nov 14, 2025
8 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.

3 participants