Skip to content

Remove Sample Rate Rounding#1073

Open
i-am-sijia wants to merge 2 commits into
ActivitySim:mainfrom
wsp-sag:remove-sample-rate-rounding
Open

Remove Sample Rate Rounding#1073
i-am-sijia wants to merge 2 commits into
ActivitySim:mainfrom
wsp-sag:remove-sample-rate-rounding

Conversation

@i-am-sijia
Copy link
Copy Markdown
Member

@i-am-sijia i-am-sijia commented Apr 30, 2026

This pull request makes a small but important change to the calculation of sample_rate in the households table logic. Instead of rounding sample_rate to three decimal places, which can result in a value of 0 for small sample sizes, the code now preserves the full precision of the division. This ensures meaningful values (not NaN or Inf) in trip matrices that use the inverse of the sample rate in their calculations.

Code changes: a simple change in the households.py.

Test updates:

Copy link
Copy Markdown
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 updates how sample_rate is computed for the households table by removing the previous rounding to 3 decimal places, preserving full precision to avoid zero sample rates for small household samples (which can cause 1 / sample_rate to overflow in downstream trip matrix logic).

Changes:

  • Remove rounding from sample_rate calculation in activitysim/abm/tables/households.py.
  • Add a new unit test validating sample_rate calculation for small sampling rates.

Reviewed changes

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

File Description
activitysim/abm/tables/households.py Computes sample_rate using full-precision division instead of rounding.
activitysim/abm/test/test_misc/test_sample_rate.py Adds a regression/unit test covering small sample_rate values.

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

Comment thread activitysim/abm/tables/households.py
Comment on lines +39 to +40
"household_id": range(1, 100001),
"home_zone_id": [1, 2] * 50000,
Comment on lines +53 to +54
assert (
sample_rate == 0.00002
@jpn--
Copy link
Copy Markdown
Member

jpn-- commented May 30, 2026

@i-am-sijia A couple of the copilot code review changes (the ones I did not mark "resolved") look useful to me. I don't have write access to your fork so the "apply changes" button doesn't work for me. Can you review and fix if you agree these are helpful? Thanks

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