Skip to content

Skim name conflicts #939

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

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

Conversation

jpn--
Copy link
Member

@jpn-- jpn-- commented May 12, 2025

The SkimDataset structure requires every variable to have a unique name. It also merges OMX variables based on time period, so that e.g. BIKETIME__AM and BIKETIME__PM, which would be 2-d arrays in the OMX file, become just two different parts of a 3-d array called BIKETIME in the SkimDataset. This is problematic when the skims also contain a 2-d array called BIKETIME, as that has no temporal dimension, and it gets loaded into a 2-d array in the SkimDataset, with the same name as the 3-d array, and thus one is overwritten and lost.

A workaround was added via the omx_ignore_patterns setting in #867, which allows the user to not load certain skims, but this relies on the model developer or user to set this up correctly.

This PR includes a skims input check to identify this overwriting condition, and raise an error if it is happening, so that the user can correct the condition via (1) the omx_ignore_patterns setting, (2) revising the skim generation process to not create the overlapping named skims in the file in the first place, or (3) renaming one or both skims if the users actually wants both skims variables in the model. The error message generated includes a link to instructions and discussion of these alternatives.

Enhancements to Skim Dataset Handling:

  • Added conflict detection for skim variables with both time-dependent and time-agnostic versions. If conflicts are found, the system now raises an error with detailed instructions for resolution. (activitysim/core/skim_dataset.py, activitysim/core/skim_dict_factory.py) [1] [2]
  • Introduced a skim_conflicts attribute in the skim dictionary factory to track and warn about duplicate skim names and conflicts. (activitysim/core/skim_dict_factory.py) [1] [2]
  • Enhanced checks for OMX file consistency, including verifying matrix shapes and ensuring unique skim names across files. (activitysim/core/skim_dict_factory.py) [1] [2]

Workflow and Dependency Updates:

  • Updated the GitHub Actions workflow to allow manual triggering via workflow_dispatch while maintaining existing conditions for automatic builds. (.github/workflows/branch-docs.yml)
  • Added re and defaultdict imports to support new functionality for skim conflict detection and regex-based ignore rules. (activitysim/core/skim_dataset.py, activitysim/core/skim_dict_factory.py) [1] [2]

Documentation Updates:

  • Added a new section on skims to the user guide, explaining their structure, usage, and naming conventions. Highlighted the deprecation of the three-zone system and provided recommendations for resolving skim conflicts. (docs/users-guide/model_anatomy.rst) [1] [2]

These changes aim to improve the robustness of skim data handling, provide clearer guidance to users, and ensure compatibility with future updates.

@jpn-- jpn-- requested a review from dhensle May 13, 2025 18:15
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