Repository navigation
Conversation
build_from_dataset sized the default-role fallback by the group IDs, so a dataset without role columns got one members_role entry per group. Role queries on such a population then raised (nb_persons, sum with role=) or broadcast one group's entry over everyone (project, max with role=). Size the fallback by the membership array, and resolve the default against each group entity: a role it names, the first subrole of a role with subroles it names, or else the entity's first role, as people given no role in a situation get. join_with_persons now rejects a role array whose length differs from the persons'. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
MaxGhenis
added a commit
that referenced
this pull request
Oct 6, 2026
Addresses the independent review of d647f78 (three should-fix findings). - group_positions compared IDs through np.searchsorted, which promotes mixed dtypes. int64 with float64, or int64 with uint64, meet as float64, where neighbouring large IDs become equal. Groups [2**53+3, 2**53+4] with a float64 person ID 2**53+4 still joined the first group, and int64 IDs [2**53, 2**53+1] with uint64 memberships raised as undeclared. IDs of one kind still use the sorted search (exact within a kind); IDs of different kinds, or object arrays, are now matched one by one through a dict, which compares Python values exactly. - Flat files with a household_id column but no person_household_id column worked on master and raised KeyError here. The membership now comes from the person's membership column or, failing that, the group's ID column repeated on each member's row. A frame with neither raises the same KeyError as master. - Default roles were sized by the number of groups. With the flat file's household count now right, two persons in one household got one role and role-filtered sums raised. They are now one per person (as in #595). Adds regressions for each, and a Hypothesis property: numeric IDs near 0, 2**31, 2**53 and 2**62 match by value for int32/int64/uint64/float64 pairs. All fail on d647f78; the property shrinks to int64 groups and uint64 memberships near 2**62. The real-dataset mapping A/B (populace-us, enhanced CPS 2024, UK enhanced FRS 1.40.3 and 1.57.4) is still identical. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This branch has not been deployed
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.
Summary
Relative to current
master, this PR rejects recognized implicit dataset defaults that exceed a role's finite per-group capacity and rejects non-vector or wrong-length role arrays during joining. Master already assigns one literal default key per person. Accepted defaults keep their role meaning, cardinality, query results, and serialization.This round integrates the overlapping change from merged #573 through a plain merge of
gh/masterat8fdf59e95a2faa990ed179f8554b722978024836. It preserves master's person-count default sizing and detection of period-suffixed entity role columns, and adds regression coverage for their precedence. The caller comparison now uses this master baseline. Capacity enforcement newly rejects some datasets whose queries previously succeeded; it does more than move an existing failure earlier. The fallback that selected a first role or subrole was removed in the previous round and remains removed.Caller semantics
Unless stated otherwise, these examples use the country template's household roles and no role column.
master[1,2,3], household[10], memberships[10,10,10], default"child";nb_persons(role=CHILD)CHILDroles;[3][3]"member"or compound key"parent"; parent count and parent sum of[1,2,3][0,0,0]; count[0], sum[0]0 + 0 + 0 = 0[1,2], households[10,20], memberships[10,10], default"first_parent"; unique-role lookup of[100,200]ValueErrorValueErrorbecause2 > 1memberrole withmax=2; persons[1,2,3], group[10], memberships[10,10,10]; member count and filtered sum of[1,2,3][3], sum[6], since1 + 2 + 3 = 6ValueErrorbecause3 > 2; no query runs["child"]project(np.array([7]), role=CHILD)broadcasts to[7,7,7]ValueError; non-vector arrays are also rejectedEntity-specific role columns, including period-suffixed flat-file columns, still override generic
role, which overrides the implicit default. Explicit integer indices and literal strings retain their interpretation.default_role=Nonestill requires a role column; one global default key still applies to every group entity. Well-shaped explicit dataset assignments, direct-join capacity handling, situation defaults, and situation capacity validation remain unchanged. Only implicit defaults receive the new capacity check; excessive explicit capacities are not newly rejected.The join guard checks role-array dimensionality and equality with the supplied membership array's length. It does not independently validate membership dimensionality, membership IDs, or agreement with the declared person count. Preserving master's person-count default sizing means a malformed implicit dataset with a different membership count is rejected by this role-length check.
Methodology
These are Core simulation contracts. No statute, regulation, form, or tax-policy instruction applies. Primary sources are master's defaults and role-column precedence, literal flattened-role matching, the capacity descriptor, subrole uniqueness, situation capacity enforcement, and the unique-role lookup contract.
Invariants and coverage
For well-formed datasets without role columns, with defined group roles, string default keys, and one-dimensional membership arrays containing exactly one declared group ID per person, across
ARRAYS,TIME_PERIOD_ARRAYS, andFLAT_FILE:len(members_role) == len(members_entity_id) == number of persons, including zero persons and empty declared groups.Role.max. Capacities zero, one, and greater than one have coverage.For datasets with explicit roles, entity roles take precedence over generic roles and defaults, including period-suffixed flat-file columns. The new Hypothesis property verifies this with integer and string encodings, conflicting generic roles, and repeated unique explicit roles whose capacity handling remains unchanged.
Hypothesis generated 200 default-role/capacity/query cases, 50 malformed-role-shape cases, and 100 suffix-precedence cases, plus eight fixed examples. Its default-query generator places empty groups after occupied groups in sorted ID order; other placements expose the separate existing join-identity defect discussed in #592. Flat-file cases use consecutive occupied IDs to isolate the loader's synthetic-ID convention. These properties do not claim membership validation or correctness for other empty-group placements or ID renumbering.
Validation
Every file below ran separately in the foreground with Python 3.13.9 through
uv run --no-sync. A workspace-local runner reused preinstalled hub dependencies read-only while the workspace dependency install completed; tested Core source came from this checkout, or the pinned master export for baseline witnesses.tests/core/test_dataset_default_role.py: 35 passed.tests/core/test_dataset_default_role_property.py: 3 passed, with 350 passing generated cases and eight fixed examples.tests/core/test_simulation_builder.py: 35 passed.tests/core/test_bincount_minlength.py: 2 passed.tests/core/test_entities.py: 23 passed.tests/core/test_projectors.py: 6 passed.tests/core/test_dump_restore.py: 1 passed.tests/core/test_dataset_unknown_column_warning.py: 2 passed.9ba5d19head, both new suffix regressions failed: absent generic roles caused the capacity error; conflicting generic roles incorrectly won. Both pass after the merge in the complete example file. The merge preview and actual merge both reproduced the default-branch conflict.ruff format --check .: 397 files already formatted;ruff check .: passed.git diff --check: passed.No full suite, microsimulation, or documentation build was run locally for this focused round. API documentation uses autodoc; the loader and join source docstrings describe the durable contract. The towncrier fragment now describes the residual capacity and shape changes relative to current master. Documentation review: impact medium, confidence high from source inspection, baseline witnesses, and focused coverage; the separate join-identity and zero-person reduction limitations remain unchanged.