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

Changes to support sharrow on 2-zone model #867

Open
wants to merge 43 commits into
base: main
Choose a base branch
from

Conversation

jpn--
Copy link
Member

@jpn-- jpn-- commented May 6, 2024

This PR has a number of changes made in support of sharrow on the new "canonical" 2-zone example, although as it turns out none of these changes really alter how ActivitySim runs...

  • Add land_use_columns_orig setting to aggregate accessibility, to allow joins against both destination (previously available) and origin (new) land use attributes without convoluted hacks in the spec.
  • Add omx_ignore_patterns to State.settings, which allows us to ignore loading certain skim tables, either because they are problematic or just to save memory.
  • Update the minimum version for sharrow to 2.9.1 (for omx_ignore_patterns) and numba to 0.57 (to access np.nan_to_num as a numba function).
  • Improve runtime logging of sharrow and non-sharrow code to disambiguate interaction-simulate steps from simulate steps.
  • Improve debugging script that is activated in interaction-simulate steps only in sharrow test mode, when sharrow and non-sharrow utility functions differ.
  • Cleanly close opened OMX files instead of leaving open file handles lingering to be closed when Python shuts down.
  • Documentation updates
  • Change to test configs for joint_tour_frequency_composition to correct math errors in the model spec.

This is useful if you have tables in your OMX file that you don't want to
read in. For example, if you have both time-of-day values and time-independent
values (e.g., "BIKE_TIME" and "BIKE_TIME__AM"), you can ignore the time-of-day
values by setting this to ["BIKE_TIME__.+"].
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this required for sharrow? If so, I think we need to put an assert statement in the skim read for sharrow somewhere that will prevent this from happening and provide some info to the user about the error.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, not for sharrow per se, but the SkimDataset converts all the by-time-of-day skim variables into three dimensional arrays (o,d,time). So without this, in the example the OMX files essentially have two different sets of data for the same named variable, one two-dimension BIKE_TIME, and one three-dimension BIKE_TIME, which coincidentally happens to have no variance across the temporal dimension. We can't have two variables in the same namespace with the same name, so one is overwritten.

Copy link
Member Author

Choose a reason for hiding this comment

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

Opened an issue #873 to address this.

This was referenced May 22, 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.

None yet

3 participants