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

Overflow protection #679

Closed
wants to merge 216 commits into from
Closed

Conversation

jpn--
Copy link
Member

@jpn-- jpn-- commented May 17, 2023

The PR adds over- and under-flow protection to the activitysim.core.logit.utils_to_probs function. This error was initially reported by @stefancoe in the PSRC model's mandatory tour scheduling.

  • Code developed, all existing tests pass
  • PSRC to confirm this resolves their current error.
  • Write new unit test to ensure utility shifting works for various corner cases
  • Test for performance on at least a couple full scale models to confirm the computational overhead is acceptably small

The default is now to shift utility values such that the maximum utility in each row is zero, unless allow_zero_probs is set (the overflow protection will spoil this as zero probs are triggered by having an intentional underflow). This constant per-row shift should not fundamentally alter the computed probabilities, but will ensure that an over/underflow does not occur that will create infinite or NaN values. Extremely rare probabilities will round to zero, but by definition they are extremely rare and losing them entirely should not impact the simulation in a measurable fashion, and at least one (and sometimes only one) alternative is guaranteed to have non-zero probability, as long as at least one alternative has a finite utility value. If utility values are certain to be well-behaved and non-extreme, enabling overflow_protection will have no benefit but impose a modest computational overhead cost, but the possibility of encountering a show-stopping underflow accidentally as a corner case probably makes this overhead worthwhile all the time.

Note this PR looks big because it's on top of the orca PR, it's actually relatively simple see here for the relevant diff.

@jpn-- jpn-- marked this pull request as draft May 17, 2023 22:38
@jpn-- jpn-- marked this pull request as ready for review May 18, 2023 00:10
@jpn-- jpn-- requested a review from stefancoe May 18, 2023 00:15
Copy link
Contributor

@stefancoe stefancoe left a comment

Choose a reason for hiding this comment

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

This fixed the overflow issue we were having in mandatory tour scheduling.

@jpn-- jpn-- mentioned this pull request Dec 15, 2023
4 tasks
@jpn--
Copy link
Member Author

jpn-- commented Dec 15, 2023

See #764 instead

@jpn-- jpn-- closed this Dec 15, 2023
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.

2 participants