Finalise DANRA notebook: merge-artifact fixes + polish (#577)#1
Merged
Sharkyii merged 11 commits intoJul 16, 2026
Merged
Conversation
Co-authored-by: Hurricane <hurricane@hermes.local>
- Route training and checkpoint reload through build_predictor - Omit hierarchical mesh GNN kwargs for graph_lam - Remove GraphLAM **_kwargs swallow - Add regression test for graph_lam kwargs
Round-2 review of the DANRA notebook PR. Most of these are regressions from merging main into the stale branch, not issues with the original work. - workflow: the `Run tests (excluding notebooks)` step had no `run:` and every new step gated on a non-existent `matrix.package_manager`, so `CPU+GPU testing` failed to parse (0 jobs) and the notebook CI this PR adds never ran. Collapse to a plain test step plus a push/label-gated notebook step. - notebook cell 18: pass the now-required `mesh_node_features_scaling` to `utils.load_graph` (introduced in mllam#323), which otherwise TypeErrors. - CHANGELOG: move the entries out of the frozen v0.6.0 block (they created duplicate `### Added`/`### Changed` headers) into a single consolidated `[unreleased]` entry. - pyproject: restore the `[tool.hatch.build.targets.sdist]` exclude that was dropped when `[tool.pytest.ini_options]` was moved. - train_model: import `MODELS` from `neural_lam.models` instead of redefining the registry locally. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Address review of the DANRA hello-world notebook: - Install: replace the executable uv-venv / pip cells (Option A/B) with a single markdown instruction using the README's `uv sync --extra cpu --group dev`. The old `uv venv --no-project` cell recreated the repo's .venv, and the parallel pip path was redundant. As markdown it no longer mutates the running env and is skipped by nbmake in CI. - Graph viz (cell 18): the plotly 3D figure needs WebGL and fails to render inline (e.g. in VSCode). Keep writing the interactive graph_viz.html for the browser, and add a lightweight static matplotlib 2D preview (mesh nodes + grid nodes + M2M edges only, dense G2M/M2G skipped) that renders anywhere without WebGL. - Versions: Python note 3.10-3.12 -> 3.10-3.14; GPU scaling tip and the mllam-data-prep hint use uv (`uv sync --extra gpu`, cu130) instead of the stale `pip install torch ... cu121`. Verified: notebook passes `pytest --nbmake` end-to-end (52s). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
sadamov
changed the title
Pull in @gitcommit90's fix for the graph_lam GNN kwarg bug (mllam#688, closes mllam#686): train_model passed hierarchical-only mesh_up/mesh_down gnn_type kwargs to every model, so `--model graph_lam` crashed with a TypeError. mllam#688 adds a `build_predictor` helper that only forwards those kwargs to hi_lam / hi_lam_parallel. This is carried here only to give the notebook a working training run. mllam#688 is the canonical fix - merge it first, then rebase/merge mllam#577 on top. Conflict resolved in tests/test_train_model_warnings.py: kept both main's test_steps_to_log_validation and mllam#688's new gnn-kwarg tests.
Make the DANRA notebook run end-to-end and render everywhere: - Run training/eval via subprocess.run(..., check=True) instead of the `!` shell escape, which silently swallowed non-zero exits (training was failing invisibly in CI and local renders). - Set MPLBACKEND=Agg for the train/eval subprocesses. The default interactive TkAgg backend aborts (Tcl_AsyncDelete, surfacing as a "double free" SIGABRT) when Lightning plots from callback threads; Agg is non-interactive and stable. - Fix stale output paths after the runs/ refactor: checkpoints under runs/<run>/checkpoints (glob runs/**/*.ckpt, raise clearly if none), eval plots under runs/<run>/wandb/.../media/images (glob runs/**/test_rmse_*.png and runs/**/*_example_*.png). - Graph viz: static matplotlib 3D preview (grid + mesh layers, no WebGL so it renders inline in VSCode) with the interactive plotly 3D below it, and the full interactive graph saved to graph_viz.html. - num_workers 0 for the single-process CPU hello-world. Verified end-to-end with pytest --nbmake (train + eval + plots, no crash). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…errors - Add .github/workflows/notebook-tests.yml: runs docs/notebooks/ via nbmake on every PR that touches neural_lam/**, docs/notebooks/**, the datastore fixtures or the lockfile (plus push to main and manual dispatch), so notebook rot from API drift is caught before merge instead of after. CPU-only, and no longer double-runs on the gpu matrix leg like the old in-job step did. - Drop the label-gated notebook step from install-and-test.yml. - train_model.main: use @logger.catch(reraise=True) so a Python error propagates (non-zero exit) instead of being logged and swallowed. Without this the notebook's subprocess.run(check=True) could not detect train/eval failures. Update the test mock to accept the parametrised decorator form. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Thanks @Sharkyii, notebook looks good. This branch takes it to merge-ready.
Merge-artifact fixes (regressions from merging main into the branch, not your original work):
matrix.package_manager(this workflow is uv-only) andRun tests (excluding notebooks)had norun:, soCPU+GPU testingfailed with 0 jobs and notebook CI never ran. Collapsed to a plain test step + a push/label-gated notebook step.utils.load_graphnow needsmesh_node_features_scalingafter [Feat] Add graph-on-disk format specification and validator mllam/neural-lam#323 (credit @GiGiKoneti from Add DANRA tutorial notebook with pytest-nbmake mllam/neural-lam#705).[unreleased].[tool.hatch.build.targets.sdist]exclude; train_model importsMODELSfromneural_lam.modelsinstead of redefining it.Notebook polish:
uv sync --extra cpu --group dev(markdown, matching the README) instead of theuv venv --no-project+ parallel pip cells. As markdown it no longer mutates the running env and is skipped by nbmake.graph_viz.htmland added a light static matplotlib 3D preview (mesh + grid nodes + M2M edges).uv sync --extra gpu/ cu130 instead of the stale cu121.Verified the notebook passes
pytest --nbmakeend-to-end locally (52s).