Skip to content

Support reading skims from Parquet files, in addition to OMX - #6

Draft
jpn-- with Copilot wants to merge 6 commits into
mainfrom
copilot/read-skims-from-parquet
Draft

Support reading skims from Parquet files, in addition to OMX#6
jpn-- with Copilot wants to merge 6 commits into
mainfrom
copilot/read-skims-from-parquet

Conversation

Copilot AI commented Jul 21, 2026

Copy link
Copy Markdown

Skims can currently only be read from OMX files. Parquet storage is significantly more compact (~30% smaller) and can be read faster, so this adds Parquet as an alternative input format, auto-detected per-file, with full backward compatibility for OMX.

New Parquet skim reader (activitysim/core/skim_parquet.py)

  • ParquetSkimFile inspects a parquet skim file's first two columns (origin, destination) to derive the zone id set and matrix shape; remaining columns are treated as named skim matrices, using the same naming/time-period conventions as OMX (e.g. DIST, DRV_COM_WLK_BOARDS__AM).
  • Detects whether rows are dense (every OD pair present) and sorted row-major or column-major, using a fast reshape path in that case.
  • Falls back to a sparse read path (scatter into a dense matrix) for incomplete or unsorted-dense-but-actually-sparse data; sparse data may be sorted or unsorted.
  • Raises ValueError if data is dense (complete OD coverage) but not sorted row-major/column-major, since it can't be read efficiently or unambiguously in that state.

Integration (activitysim/core/skim_dict_factory.py)

  • SkimInfo.load_skim_info and _read_skims_from_omx now branch per-file on extension (.parquet/.pq vs OMX), so OMX and Parquet files can be freely mixed under the same skim tag.
  • Each file's zone ids/layout are validated independently — files are not assumed to share row ordering — while shape and zone id sets are still checked for consistency across all files for a given skim tag.
  • No changes needed in los.py: users just reference .parquet filenames directly in network_los.yaml, e.g.:
taz_skims:
  - z1_taz_skims_part1.parquet
  - z1_taz_skims_part2.parquet

Scope

  • Applies to the legacy NumpyArraySkimFactory/MemMapSkimFactory path (SkimDict).
  • The sharrow-based SkimDataset path still relies on the external sharrow package's sh.dataset.from_omx_3d and does not yet support Parquet input; extending that would require changes upstream in sharrow.

Docs

  • TAZ_Settings.omx and SkimInfo docstrings updated to describe Parquet input expectations (dense/sparse layout, column conventions).

Copilot AI linked an issue Jul 21, 2026 that may be closed by this pull request
Copilot AI changed the title [WIP] Add support for reading skims from parquet format Support reading skims from Parquet files, in addition to OMX Jul 21, 2026
Copilot finished work on behalf of jpn-- July 21, 2026 17:20
Copilot AI requested a review from jpn-- July 21, 2026 17:20
Comment thread activitysim/core/skim_parquet.py Outdated
):
return ROW_MAJOR

col_major_orig = np.tile(np.arange(n), n)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

col_major_orig is the same as row_major_dest, there is no need to create this array twice.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Fixed in 288c238 — col-major orig/dest now reuse row_major_dest/row_major_orig instead of recreating equivalent arrays.

Comment thread activitysim/core/skim_parquet.py Outdated
return ROW_MAJOR

col_major_orig = np.tile(np.arange(n), n)
col_major_dest = np.repeat(np.arange(n), n)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

col_major_dest is the same as row_major_orig, there is no need to create this array twice.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Fixed in 288c238 — same fix, reusing row_major_orig/row_major_dest for the col-major check.

Copilot finished work on behalf of jpn-- July 26, 2026 17:35
Copilot AI requested a review from jpn-- July 26, 2026 17:35

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Adds Parquet as an additional skim storage/input format for the legacy SkimDict pipeline, auto-detected per file by extension, and extends tests/config fixtures to validate Parquet-based LOS loading.

Changes:

  • Introduce activitysim/core/skim_parquet.py with ParquetSkimFile for inspecting and reading Parquet skim tables (dense row/col-major and sparse).
  • Extend SkimInfo.load_skim_info / _read_skims_from_omx to mix OMX and Parquet skim files under a single skim tag.
  • Add unit/integration tests and canonical test configs validating Parquet skim loading (single file and multi-file scenarios).

Reviewed changes

Copilot reviewed 9 out of 12 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
activitysim/core/skim_parquet.py New Parquet skim reader/inspector used by legacy skim loading.
activitysim/core/skim_dict_factory.py Adds Parquet branching + caching during skim discovery and loading.
activitysim/core/configuration/network.py Documents Parquet skim input expectations under TAZ_Settings.
activitysim/core/test/test_skim_parquet.py New unit tests for dense/sparse Parquet layout detection and reading.
activitysim/core/test/test_los.py New LOS integration tests exercising Parquet skim inputs.
activitysim/core/test/los/configs_1z_parquet/settings.yaml New canonical config for 1-zone Parquet skim test.
activitysim/core/test/los/configs_1z_parquet/network_los.yaml New network LOS config pointing to .parquet skims.
activitysim/core/test/los/configs_1z_parquet_multi/settings.yaml New canonical config for multi-file Parquet skim test.
activitysim/core/test/los/configs_1z_parquet_multi/network_los.yaml New LOS config pointing to two Parquet skim parts.
Comments suppressed due to low confidence (1)

activitysim/core/skim_parquet.py:138

  • Same as above: pyarrow.Table.column(...) is not reliably name-addressable across the supported pyarrow range. Indexing the table by column name is the stable API.
        values = table.column(column_name).to_numpy(zero_copy_only=False)
        if dtype is not None:

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

Comment on lines +70 to +87
zone_ids = np.unique(np.concatenate([origins, destinations]))
self.zone_ids = zone_ids
self.n_zones = len(zone_ids)

self.shape = (self.n_zones, self.n_zones)

n_rows = len(origins)
self.is_dense = n_rows == self.n_zones * self.n_zones

zone_index = {z: i for i, z in enumerate(zone_ids)}
orig_idx = np.fromiter(
(zone_index[o] for o in origins), dtype=np.int64, count=n_rows
)
dest_idx = np.fromiter(
(zone_index[d] for d in destinations), dtype=np.int64, count=n_rows
)
self._orig_idx = orig_idx
self._dest_idx = dest_idx
Comment thread activitysim/core/configuration/network.py
Comment on lines +386 to +390
if is_parquet_file(omx_file_path):
parquet_skim_file = skim_info.parquet_files.get(omx_file_path)
if parquet_skim_file is None:
parquet_skim_file = ParquetSkimFile(omx_file_path)
for skim_key, omx_key in omx_keys.items():
Comment thread activitysim/core/skim_parquet.py Outdated
Copilot finished work on behalf of jpn-- July 29, 2026 16:55
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.

read skims from parquet

3 participants