Stabilization: modernize deps and fix the instabilities that surfaced - #219
Open
nick-fournier wants to merge 19 commits into
Open
Stabilization: modernize deps and fix the instabilities that surfaced#219nick-fournier wants to merge 19 commits into
nick-fournier wants to merge 19 commits into
Conversation
Author
nick-fournier
marked this pull request as ready for review
July 9, 2026 23:23
nick-fournier
marked this pull request as draft
July 9, 2026 23:25
nick-fournier
marked this pull request as ready for review
July 9, 2026 23:25
nick-fournier
changed the title
There was a problem hiding this comment.
Pull request overview
Updates the project’s Python dependency floor/ceiling and dependency versions to support newer NumPy and OR-Tools, and adjusts packaging/CI configuration to match the updated Python ecosystem targets.
Changes:
- Bumped core dependencies (NumPy to 2.x, OR-Tools to 9.14.x) and expanded the supported Python range to include 3.13.
- Updated GitHub Actions CI to run tests on Python 3.10–3.13.
- Restricted setuptools package discovery to
populationsim*packages.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| pyproject.toml | Updates Python version constraints, bumps NumPy/OR-Tools dependencies, and narrows package discovery to populationsim*. |
| .github/workflows/python-package.yml | Updates the CI Python test matrix to include Python 3.13 and drop 3.9. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…im into deps/numpy_2.x
nick-fournier
marked this pull request as draft
August 13, 2026 18:41
`dtypes: {col: str}` in a table_list entry routed through astype(str),
whose missing-value semantics changed between pandas majors: pandas 2
produces the literal string "nan", pandas 3's string dtype preserves
NA. Control expressions that test for missing values therefore
evaluated differently depending on the installed pandas.
example_survey_weighting hits this directly. It declares `PComm: str`
and defines the PComm_n control as `persons.PComm.isna()`, which sums
to 0 under pandas 2 and 29682 under pandas 3 -- an entire control
column silently dropped, shifting balanced weights by ~0.4%. That is
what made test_weighting pass on python 3.11+ and fail on 3.9/3.10,
which resolve pandas 2.x. It is a live data bug, not only a test
artifact: any run with a str-typed column containing blanks silently
loses its .isna() controls.
Restore NA explicitly after string casts so results no longer depend
on the installed pandas major.
test_weighting's np.allclose had no assert, so the comparison never
ran. Add it, and regenerate weights.parquet: the stored baseline had
recorded the pandas 2 result with PComm_n empty, so the baseline
asserted the bug. With the cast fixed, agreement is 5e-14 relative and
bit-identical across pandas majors, so the tolerance is set to 1e-8
rather than carrying a loose one.
Verified: full suite passes on python 3.10 / pandas 2.3.3 and on
python 3.12 / pandas 3.0.3.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
tests/test_rng.py pins the surfaces the pipeline actually relies on: the external rng used by expand_households to pick seed households, and the per-step global rng. Seeded Mersenne Twister is deterministic on every platform, so unlike the expanded-household comparison these assert an exact known sequence. A double-run test also confirms the pipeline is bit-for-bit reproducible on a single machine, which the cross-platform comparator in tests/regression.py deliberately cannot check. tests/generate_expected.py regenerates the tests/expected baselines. Doing that by hand is error-prone, and a stale baseline silently encodes a bug rather than catching one -- as weights.parquet did. Guard close_pipeline() with is_open() in the test teardowns. The new double-run test opens and closes pipelines mid-session, so a teardown that assumes an open pipeline raises and masks the real result. Both new files are taken from the allow_intermediate_geographies branch, with their docstrings repointed at tests/regression.py. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
docs/conf.py opened ../setup.py to scrape the version string, but setup.py was removed when the project moved to pyproject.toml, so the sphinx build raised FileNotFoundError before reaching any content. Read it from installed package metadata instead. Also ignore .venv/. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
test_integerizer calls config.override_setting, which replaces the decorated `settings` injectable with a plain dict. clear_cache() does not restore a decorated injectable -- only reinject_decorated_tables() does -- and this module had no teardown, so every test running after it inherited example_test/configs settings for the rest of the session. That has been harmless only by coincidence: every other test happens to use the same example, so the stale `geographies` list matched. Any test introducing a different geography set fails in the suite while passing in isolation, which is the most expensive kind of failure to diagnose. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
nick-fournier
changed the title
Sub-ULP floating-point noise between platforms was enough to flip which households smart_round selects, even though the LP's objective value -- and therefore which zone counts are hit -- is unaffected. The mechanism to fix this already existed (INTEGERIZER_QUANTUM, tie_break_by_position) but defaulted to off, so it fixed nothing unless a config explicitly opted in. Measured with tests/determinism_probe.py: with quantization on, household selection is unaffected by input perturbations up to 1e-9 relative; zone counts hold to 1e-8. Real cross-version float noise measured elsewhere in this codebase (the pandas NA fix's before/after comparison) is ~5e-14 relative -- five orders of magnitude inside that margin. This closes the part of the problem within this codebase's control. It does not address whether CBC's own compiled binary makes an independent platform-dependent choice among tied-optimal LP solutions given identical input; that is upstream of anything this repo can fix by construction. The new CI step prints an unperturbed determinism fingerprint per job so that can be checked directly, by comparing the hash across this matrix's 6 platforms once CI completes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
uv run python <script> does not add the project root to sys.path the way pytest's own rootdir insertion does, so the previous commit's new step failed with ModuleNotFoundError on all 6 platforms. The actual test suite passed everywhere in that run -- only this step's own invocation was broken. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
nick-fournier
marked this pull request as ready for review
August 18, 2026 19:30
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Scope of this PR shifted a bit. In the process of updating numpy and ORTOOLS I ran into the non-deterministic results issue again. I am pretty confident with this solution. It is definitely stable across runs on the same machine, I am confident it is stable across machines/OS but not guaranteed. It at least passes on the small test case we have, but its possible the quantization trick breaks down on something bigger.
This should unblock @JoeJimFlood's intermediate geography PR, but that PR is a bit bloated with my failed patch fixes and so I propose closing it and reopen a tiny targeted PR.
Root issue: Not solver randomness. A
strcast handled missing values differently between pandas 2 and 3, silently dropping a control on pandas 2 and shifting results. Fixed, baseline regenerated.Caveats: Separately, the solver can pick different-but-equally-valid households under tiny float noise. Mitigated with
INTEGERIZER_QUANTUM(now default on) — confirmed identical output across all 6 CI platforms, but that's one example, one run, not a formal guarantee.Changelog:
test_weightingINTEGERIZER_QUANTUMdefault ontest_integerizerINTEGERIZER_TIMEOUTtypo (relevant to Randomness of PopulationSim outputs related to API calls #182)