Skip to content

Add Python visualization notebooks - #516

Draft
pelesh wants to merge 3 commits into
developfrom
uq-notebooks-reorg
Draft

Add Python visualization notebooks#516
pelesh wants to merge 3 commits into
developfrom
uq-notebooks-reorg

Conversation

@pelesh

@pelesh pelesh commented Aug 1, 2026

Copy link
Copy Markdown
Collaborator

Description

Reorganizes the Python geo-visualization notebook and utilities proposed in #490 (m_viz: geo visualization for Hawaii40, Illinois, Texas, WECC) to match GridKit's existing directory conventions, and adds CMake wiring and unit tests for it.

This PR keeps the same notebook and the same visualization functionality, but reorganizes it before merge.

Supersedes #490.

CC @isatkaus

Proposed changes

  • Moves the notebook and Python utilities to application/notebooks/, matching the existing application/PhasorDynamics convention.
  • Splits m_viz_utils.py (1220 lines) into application/notebooks/m_viz/{parsing,geo,plotting}.py.
  • Splits gridkit_utils.py (641 lines) into:
    • application/notebooks/gridkit_common/ids.py — the two functions (attach_json_ids, get_case_path_for_editing) the notebook actually uses.
    • application/notebooks/uq_sampling/{sampling,runs,dispatch}.py — the UQ parameter-sampling, run-orchestration, and Genrou dispatch-patching code that was bundled in but unused by this notebook. Kept in this PR, split by concern, and documented as not yet wired to a notebook in this repo.
  • Adds application/notebooks/pyproject.toml, making the code an installable package (pip install -e .) and removing the notebook's sys.path.insert hack.
  • Adds tests/UnitTests/Python/ with 18 pytest unit tests for m_viz.parsing and m_viz.geo (previously untested).
  • Adds CMake wiring: a new GridKit_ENABLE_NOTEBOOKS option (alongside the existing GridKit_ENABLE_* flags) gates add_subdirectory(application/notebooks); a notebooks_pyenv custom target creates a venv and installs the package; the ctest Python suite is gated the same way and runs against that venv, mirroring the existing test_install before/after build ordering. Documented in INSTALL.md.
  • Replaces the hardcoded personal paths in the notebook with environment-variable-driven configuration (GRIDKIT_CASE_DATA_DIR, GRIDKIT_REPO, GRIDKIT_FIGS_DIR), consistent with the notebook's existing NREL_CLUSTER/onkestrel pattern.
  • Drops _build_branch_line_arrays from the plotting module — confirmed dead code, never called from plot_grid or anywhere else.
  • No changes to the notebook's visualization output or behavior — this is a reorganization, not a rewrite.

Checklist

  • All tests pass. (18/18 pytest tests pass; verified end-to-end via ctest with -DGridKit_ENABLE_NOTEBOOKS=ON)
  • Code compiles cleanly with flags -Wall -Wpedantic -Wconversion -Wextra. (N/A — Python-only change, no C++ added or modified)
  • The new code follows GridKit™ style guidelines.
  • There are unit tests for the new code.
  • The new code is documented.
  • The feature branch is rebased with respect to the target branch. (branched directly from develop)
  • I have updated CHANGELOG.md to reflect the changes in this PR.

Further comments

This branch has two commits: the first brings in #490's content unmodified as a baseline, the second applies the reorganization — so the diff between them documents exactly what moved and what changed. See #490's own description for the visualization feature itself; nothing about that feature changed here.

pelesh and others added 2 commits August 1, 2026 14:37
Brings in the notebook, setup docs, and Python utilities proposed in
PR #490 (m_viz: geo visualization for Hawaii40, Illinois, Texas, WECC)
as a starting point for restructuring into application/notebooks/.

Co-Authored-By: isatkaus <isatkaus@users.noreply.github.com>
… out

Reorganizes PR #490 uq-usecase contribution to match the existing
directory conventions (application/, tests/UnitTests/Area):

- application/notebooks/m_viz/: parsing.py, geo.py, plotting.py split
  out of the single 1220-line m_viz_utils.py by concern.
- application/notebooks/gridkit_common/: the two functions
  (attach_json_ids, get_case_path_for_editing) that gridkit_utils.py
  actually exercised from the notebook.
- application/notebooks/uq_sampling/: the UQ parameter-sampling,
  run-orchestration, and Genrou dispatch-patching code that was bundled
  into gridkit_utils.py despite being unrelated to and unused by the
  m_viz notebook. Kept in this PR per request, split into
  sampling.py, runs.py, dispatch.py, and documented as not yet wired to
  a notebook in this repo.
- application/notebooks/pyproject.toml: installable package, replacing
  the notebook sys.path.insert hack.
- tests/UnitTests/Python: pytest unit tests for m_viz.parsing and
  m_viz.geo (previously untested).
- Wired into the C++ build as opt-in: GridKit_ENABLE_NOTEBOOKS gates
  add_subdirectory(application/notebooks), which exposes a
  notebooks_pyenv custom target that creates a venv and pip-installs the
  package. GridKit_ENABLE_NOTEBOOKS also gates the ctest Python suite,
  which runs against that venv, mirroring the existing test_install
  ordering: build notebooks_pyenv, then ctest.
- Replaced hardcoded personal paths (under /Users/isatkaus and
  /home/isatkaus) in the notebook with environment-variable-driven
  config (GRIDKIT_CASE_DATA_DIR, GRIDKIT_REPO, GRIDKIT_FIGS_DIR),
  consistent with the existing NREL_CLUSTER/onkestrel pattern already in
  the notebook.
- Dropped _build_branch_line_arrays from m_viz_utils.py: confirmed dead
  code, never called from plot_grid or anywhere else.

Verified: all modules py_compile clean, notebook JSON is well-formed,
pip install -e application/notebooks succeeds in a clean venv, all 18
pytest tests pass, and the full CMake flow (configure with
-DGridKit_ENABLE_NOTEBOOKS=ON, build notebooks_pyenv, ctest) passes
end-to-end.

Co-Authored-By: isatkaus <isatkaus@users.noreply.github.com>
@pelesh
pelesh requested a review from lukelowry August 1, 2026 19:50
@pelesh pelesh added the enhancement New feature or request label Aug 1, 2026
@lukelowry

Copy link
Copy Markdown
Collaborator

We can set this up so that the output of the notebook renders in ReadTheDocs, FYI. Out of scope but worth mentioning

@lukelowry

Copy link
Copy Markdown
Collaborator

I can run this, seems to work. I would ask that we wait until the remaining models are merged and I update the cases so this doesn't unintentionally become dead weight.

Also, I have all of the GIS coordinates of the cases that I plan on adding to the extension section of Bus so that we only have to parse our own format. Otherwise, this is useful and cool!

@pelesh

pelesh commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator Author

I can run this, seems to work. I would ask that we wait until the remaining models are merged and I update the cases so this doesn't unintentionally become dead weight.

Also, I have all of the GIS coordinates of the cases that I plan on adding to the extension section of Bus so that we only have to parse our own format. Otherwise, this is useful and cool!

Let's get release 0.2 out and then merge GIS and the notebooks. Thanks @isatkaus and @lukelowry

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants