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.

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

Fixes windows error on large MAZ systems #760

Merged
merged 4 commits into from
Dec 21, 2023

Conversation

jpn--
Copy link
Member

@jpn-- jpn-- commented Dec 1, 2023

Turns out the error in #681 has actually been a longstanding problem, since the dawn of the use of MAZ's in ActivitySim. It's just that it has only come to light recently as it requires a particular confluence of conditions to have it cause the crash that PSRC was seeing:

  1. running on Windows (hence default integer type is int32, not int64)
  2. using a two-zone system with more than 32K microzones (hence some combination of origin and destination zone index codes will overflow an int32)
  3. a specification that prohibits trip modes other than WALK when the tour mode is WALK
  4. a tour generated with a tour mode of WALK, which goes to a destination that is accessible by WALK according to the MAZ-level skims, but would not be accessible by WALK if only the TAZ level skims were used
  5. Either the origin or the destination zone index code of this tour is larger than 32768.
  6. Also, whatever zone pair happens to match the overflowed remainder must not have a valid WALK travel time defined.

Obviously this is a very narrow corner case to cause a crash. However, the bug was (slightly) more serious than that. All these conditions need to be met to cause a crash, but only a subset are needed to cause erroneous results: MAZ skim values on Windows were being incorrectly evaluated for many trips and tours where the origin or destination zone index code is larger than 32768.

This fix upcasts the index lookup values to 64 bit when necessary, solving the problem.

Many thanks to @stefancoe for his assistance in highlighting and correcting this insidious error.

@jpn-- jpn-- merged commit cb3762f into ActivitySim:develop Dec 21, 2023
18 checks passed
@jpn-- jpn-- deleted the fix-windows-2zone branch December 21, 2023 21:01
@jpn-- jpn-- mentioned this pull request Feb 14, 2024
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.

1 participant