Refactor IM calculation for lazy calculation and faster RotD - #217
Open
lispandfound wants to merge 6 commits into
Open
Refactor IM calculation for lazy calculation and faster RotD#217lispandfound wants to merge 6 commits into
lispandfound wants to merge 6 commits into
Conversation
lispandfound
added a commit
that referenced
this pull request
Sep 11, 2026
Every failure the shared workflow reports on the previous commit, plus the four that `no_parallel` was already failing before any of this. **uv.lock.** The lock pinned numba 0.53.1 (March 2021), whose llvmlite 0.36 refuses to build on anything past Python 3.9, so `uv sync` died in deptry, pytest and typecheck alike. That was the whole of the three Python job failures on #217. Relocked, and — as asked — the rest of the lock is upgraded along with it, dev tooling included: ruff 0.16.1 to 0.16.7, ty 0.0.66 to 0.0.80, hypothesis, coverage, qcore-utils 2026.6.1 to 2026.8.2, numpy 2.5.1 to 2.5.3, and the transitive set behind them. **.python-version.** uv picked the interpreter off the runner before; the retired test.yml had pinned 3.13 through setup-python, and the shared workflow has no such step. Pinning it here keeps CI and a local checkout on the same interpreter. **ruff format.** The old ruff.yml ran `ruff check` only, so two files had never been through the formatter. **cargo fmt.** Likewise never run. Edition 2024 sorts `use` groups lowercase-last and no longer indents trailing comments to the previous expression. **deptry.** dask is declared but never imported: xarray dispatches to it when `apply_ufunc(..., dask="parallelized")` is handed a dask-backed array, which is what the `lazy` extra is for. Recorded as a DEP002 ignore. **ty.** The suppression on the invalid-shape test no longer matches any diagnostic, and ty now reports the dead directive. **numpydoc.** Seven kernel helpers in IM/ims.py and one in IM/snr_calculation.py had a summary line and nothing else. Written out in the style the neighbouring `_rotd_kernel` and `_components` already use. **lefthook.yml**, adapted from ucgmsim/nzcvm, so none of the above has to be found by CI again. Three changes to that config beyond porting it: - Split into pre-commit and pre-push. Upstream runs everything on pre-push against {staged_files}, but nothing is staged at push time: those commands report "no files for inspection" and skip, while the two that do not reference the template (clippy, rustfmt) run over the whole crate on every push. Autofix now runs on pre-commit over {staged_files}, where stage_fixed can do something, and the gates run on pre-push over {push_files}. - Added ruff-format, numpydoc and rustfmt. The shared workflow gates on all three and the nzcvm config predates that; between them they cover most of what is fixed above. - Fixed the yaml glob. `*.{yaml, yml}` has a space inside the alternation, so the second branch only ever matched a file literally named `* yml` and the hook never fired on a .yml. Now `*.{yaml,yml}`. The vale command from the nzcvm config is deliberately left out — it lands with the vale config itself in the next PR, rather than pointing at a .vale.ini that does not exist yet. **.yamllint.yml**, because that yaml hook does now fire, and yamllint's defaults flag things Actions requires: no `---` document start, `on:` as a mapping key, and long `${{ }}` expressions. **.gitignore**, which had no entry for target/, *.egg-info/, .coverage, htmlcov/, __pycache__/ or any of the tool caches. With hooks operating on staged files, `git add -A` picking those up is a live hazard. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
Author
|
NB: ruff, ty, etc note passing, see the PRs on top of this stack that migrate the CI workflow to the meta action and get it to pass. I have locally run tests and they pass. |
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
1 participant
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.
As title describes. Reasonably large refactor aimed to address a few things:
Hodge-podge pandas dataframes and xarray dataset returns. Standardises now on xarray outputs because...
...IM calculation is now dask aware and can hence process larger-than-memory datasets by chunking, a strategy employed in workflow upstream.
RotD calculation significantly speed up via the use of a clever convex hull trick to optimise the calculation of RotD.
RotD calculation now also returns the orientation of the obtained RotD values. Currently RotD50
Outstanding: Determine a sensible resolution to the orientation question for RotD50 in particular (and also RotD0, RotD100).
RotD calculations
The RotD calculations for all intensity measures are significantly speed-up over the previous implementation (and to my knowledge, any implementation globally) by pre-processing the waveforms prior to calculation. The results are a significant speed-up, 100x speed increase in RotD calculations in single threaded workloads. Given the dominance of RotD calculations in the time to run IM calc this represents a hopefully significant improvement for updates in NZGMDB. The result is verified against the naive algorithm in rust using property tests against the brute force method. A tabulation of speed-ups against the baseline.
The core observation is related to the Fundamental Theorem of Linear Programming: if we treat the RotD problem as a linear program to maximise$180 \times N_t$ to $180 \times H$ where $H$ is the number of points in the hull. Typically $H << N_t$ , of the order of 16 vs tens of thousands of timesteps which explains the speed-up. Here is a claude diagram of the process that illustrates the algorithm in action.
cos(theta) * 000 + sin(theta) * 090for a fixed theta, then we get for free that our maxima must lie on the convex hull of the waveforms in 2D. Because the convex hull is invariant of theta we can re-use the same hull for every angle, and so reduce the complexity of the problem from