Rewrite bb-sim on dask and switch to the site-calculation models by lispandfound · Pull Request #130 · ucgmsim/workflow

lispandfound · 2026-08-04T01:06:44Z

As described, but I took the opportunity to further refactor bb-sim:

bb-sim is now dask powered, so requires less memory on clusters by making use of chunking.
bb-sim now handles mismatched sample rates between low frequency and high frequency via signal resampling.
bb-sim now embeds the vs30 and all the site amplification parameters used into the output.
Unified several inconsistent uses of latitude/longitude vs lat/lon which were causing cascading handling of different labels

gemini-code-assist · 2026-08-04T01:06:48Z

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

Copilot

Pull request overview

Updates the broadband simulation pipeline to use the BA2018 + CB2014 site amplification models from the site-calculation package, while refactoring bb-sim to operate lazily with dask/xarray for lower memory usage and adding metadata to outputs.

Changes:

Refactors broadband merging to dask/xarray, including station-chunked processing and per-chunk site amplification + filtering.
Adds signal resampling to handle LF/HF mismatched sample rates and embeds Vs30 + site amp parameters in output metadata.
Introduces SiteAmpModel enum and extends broadband parameter schemas/defaults to support BA2018/CB2014 selection.

Reviewed changes

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

Show a summary per file

File	Description
workflow/scripts/bb_sim.py	Reworks broadband merge to dask/xarray, adds resampling/alignment, and applies BA2018/CB2014 amplification from `site-calculation`.
workflow/schemas.py	Adds `SiteAmpModel` and tightens broadband parameter validation (including new taper params).
workflow/realisations.py	Updates `BroadbandParameters` dataclass to include `fhightop`, `fmax`, and typed `site_amp_version`.
workflow/default_parameters/v24_2_2_4/defaults.yaml	Removes per-version bb defaults (now inherited from root).
workflow/default_parameters/v24_2_2_2/defaults.yaml	Removes per-version bb defaults (now inherited from root).
workflow/default_parameters/v24_2_2_1/defaults.yaml	Removes per-version bb defaults (now inherited from root).
workflow/default_parameters/root/defaults.yaml	Adds/updates broadband defaults including BA2018 model selection and taper frequencies.
uv.lock	Adds `site-calculation` and updates lock entries accordingly.
pyproject.toml	Declares dependency on `site-calculation>=2026.7.1`.
tests/test_realisation.py	Updates broadband parameter serialization test for new fields and enum.

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

lispandfound

Code review by Claude Code (run locally, since the review action is currently broken). 10 finding(s).

Two changes to the same function body, which is why they are one commit. **Larger than memory.** `bb-sim` read both waveform files entirely into memory and looped over the three components in Python. Now LF and HF are opened lazily, the common stations are selected on the *backend* arrays before chunking -- selecting after chunking makes station reordering an all-to-all dask shuffle, which materialises the whole array -- and the recombination runs per chunk under `map_blocks`. Chunking is over stations only, so each chunk holds complete traces for resampling, alignment and filtering. `resample_signal` and `align_datasets` replace the old pad-and-align: the two legs no longer have to share a timestep, so an SW4 LF run at one dt can be combined with HF at another. `relabel_hf_components` maps HF's 090/000/ver onto LF's x/y/z once, up front. **Site amplification.** Replaces `qcore.siteamp_models.cb_amp_multi` with the `site-calculation` models, selected by `bb.site_amp_version`: CB2014 as before, or BA2018. The amplification is now constrained to an explicit [fmin, fmax] band with logarithmic tapers at both ends, which is what the two new `fhightop` and `fmax` parameters are for -- the old code had a lowpass taper only. `site_amp_version` becomes an enum rather than a free string. It was previously declared, defaulted to "2014", and read by nothing at all. The shared broadband parameters move from each defaults version into root; the versions now carry only `flo`, which is the one value that genuinely differs between them. Station-dimension coordinates -- `supergrid_depth` among them -- ride through `map_blocks` untouched, which is why the supergrid penetration is carried as a coordinate rather than a data variable. This changes broadband results: different site amplification model, and a highpass taper where there was none. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

lf-to-xarray now writes supergrid_width(_gp) rather than SW4's SGWIDTH(GP), so bb-sim was silently dropping the width. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

The site amplification models only accept float64, so a float32 vref in the HF file would fail inside the first chunk. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

This reverts commit 6715e32.

lispandfound

Review of 8e1b574b. One finding is outside this PR's diff, so it is here rather than inline:

cylc/flow.cylc:60: the cylc workflow still calls bb-sim with the old 8-positional-argument form (stations.ll, the OutBin directory, Velocity_Model, ...), while bb-sim now takes 5 and expects an LF file.

Failure scenario: running the cylc flow passes extra, wrongly typed positional arguments (a directory for LOW_FREQUENCY_WAVEFORM_FILE, which is dir_okay=False), so typer rejects the call and the broadband task fails.

lispandfound · 2026-09-25T02:24:40Z

+    # Chunk over stations only, so every chunk holds complete time
+    # series for resampling, alignment and filtering.
+    nt = max(len(lf["time"]), len(hf["time"]))
+    n_stations = max(1, TARGET_CHUNK_BYTES // (3 * nt * np.float64().itemsize))


The station chunk size is computed from the time lengths before resampling and alignment, so after resampling a chunk can hold several times TARGET_CHUNK_BYTES.

Failure scenario: An SW4 1 Hz LF at dt=0.02 that runs longer than a dt=0.005 HF: nt is max(lf_nt, hf_nt), but the resampled, aligned axis is about 4 * lf_nt. Each chunk is then up to 4x the 256 MiB target before the float64 amplification and FFT intermediates, which risks running out of memory on cluster nodes. If this moves after alignment, vs30 still has to be chunked to match the LF/HF station chunks or map_blocks rejects it.

I've decided to leave this as-is. Will address at a later point, but I haven't noticed this becoming an issue in our runs.

lispandfound · 2026-09-25T02:24:40Z

+
+    resampled_waveform = xr.apply_ufunc(
+        sp.signal.resample,
+        _taper_tail(dset["waveform"]),


_resample_signal tapers the tail of whichever dataset it resamples, so the last 5% of the LF waveform is tapered only when the LF dt differs from the HF dt.

Failure scenario: A 1 Hz SW4 run (LF dt=0.02, HF dt=0.005) has its final 5% of LF motion faded by a Hanning window in the broadband output, while the same simulation with matching dt keeps the untapered LF. The broadband output then depends on the solver's output dt, not just the physics.

I think this is acceptable given that the broadband will have to resample in the first case so it is already dependent on solver dt. There is no way around that with the way SW4 has a variable dt.

lispandfound · 2026-09-25T02:24:41Z

+    fft_freqs = np.fft.rfftfreq(n_fft, dt)
+
+    # The amplification models require float64 inputs.
+    amp = amp_model_fn(


The site_calculation amplification models raise ValueError if any single vs30 or vs30_sim value is <= 0, while NaN values are not rejected and spread silently into the waveforms.

Failure scenario: One station with a 0 or -1 sentinel Vs30 in the vs30 file aborts the whole broadband job; the old qcore cb_amp_multi path did not validate. A NaN Vs30 passes and produces an all-NaN broadband waveform for that station with no warning.

This is ok I think. We've never encountered this issue before.

Copilot AI review requested due to automatic review settings August 4, 2026 01:06

Copilot started reviewing on behalf of lispandfound August 4, 2026 01:07 View session

Copilot AI reviewed Aug 4, 2026

View reviewed changes

Comment thread workflow/scripts/bb_sim.py Outdated

Comment thread workflow/scripts/bb_sim.py Outdated

Comment thread workflow/schemas.py Outdated

Comment thread workflow/scripts/bb_sim.py Outdated

lispandfound force-pushed the ba18_bb_sim branch from 884767f to cd47e92 Compare August 31, 2026 09:09

lispandfound changed the base branch from pegasus to srf2stoch_port August 31, 2026 09:09

lispandfound changed the title ~~Update broadband simulation to use BA18 + CB14 model from site amplification package~~ Rewrite bb-sim on dask and switch to the site-calculation models Aug 31, 2026

lispandfound force-pushed the ba18_bb_sim branch 2 times, most recently from 10b1ffb to df1f207 Compare September 2, 2026 02:52

lispandfound force-pushed the ba18_bb_sim branch from df1f207 to 3ea7ac6 Compare September 4, 2026 01:44

lispandfound force-pushed the ba18_bb_sim branch from 3ea7ac6 to d31d016 Compare September 8, 2026 22:56

lispandfound force-pushed the ba18_bb_sim branch from d31d016 to cf2ea99 Compare September 9, 2026 00:09

lispandfound force-pushed the ba18_bb_sim branch 2 times, most recently from 720c102 to fcd361b Compare September 10, 2026 20:58

lispandfound force-pushed the ba18_bb_sim branch from fcd361b to 17d8dd1 Compare September 10, 2026 21:09

lispandfound force-pushed the ba18_bb_sim branch from 17d8dd1 to 35b6ec2 Compare September 22, 2026 21:49

lispandfound force-pushed the ba18_bb_sim branch from 35b6ec2 to 0dcef94 Compare September 22, 2026 21:51

lispandfound force-pushed the ba18_bb_sim branch 2 times, most recently from 5b9721a to 9441be5 Compare September 22, 2026 22:33

lispandfound force-pushed the ba18_bb_sim branch 2 times, most recently from 4d4f0d7 to b874fa5 Compare September 23, 2026 21:16

lispandfound force-pushed the ba18_bb_sim branch from b874fa5 to 37ba1cc Compare September 23, 2026 21:28

lispandfound force-pushed the ba18_bb_sim branch 2 times, most recently from b27b715 to ef58053 Compare September 23, 2026 22:51

lispandfound force-pushed the ba18_bb_sim branch from ef58053 to e34c32c Compare September 23, 2026 23:13

lispandfound commented Sep 24, 2026

View reviewed changes

lispandfound mentioned this pull request Sep 25, 2026

Divide the solver's source filter back out of the recombination #144

Closed

lispandfound and others added 13 commits September 25, 2026 14:06

fix site amp specification

42ab2a2

Use literals in bb_sim, and declare pyfftw

f063be5

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

Carry the supergrid width through bb-sim under its new names

103bf98

lf-to-xarray now writes supergrid_width(_gp) rather than SW4's SGWIDTH(GP), so bb-sim was silently dropping the width. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

simplify bb sim code

3e5df06

simplifications of the bb sim code

7a43aef

optionally apply bandwidth filtering

381d8a5

further simplifications

ad6305f

extract vs30 from reported values in hf

5882709

Cast hf-sim's vref to float64 for the amplification models

a8bc14e

The site amplification models only accept float64, so a float32 vref in the HF file would fail inside the first chunk. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

remove manual indexing loop

8e1b574

shift nt calculation to worked on resampled waveforms

6715e32

Revert "shift nt calculation to worked on resampled waveforms"

88f6853

This reverts commit 6715e32.

lispandfound commented Sep 25, 2026

View reviewed changes

lispandfound added 2 commits September 25, 2026 14:32

unify latitude longitude coordinate convention

41f028a

unify component treatment

3a00703

AndrewRidden-Harper approved these changes Sep 25, 2026

View reviewed changes

lispandfound mentioned this pull request Oct 7, 2026

Occasional crashes in broadband simulation due to duplicate stations #107

Closed

Conversation

lispandfound commented Aug 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gemini-code-assist Bot commented Aug 4, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lispandfound left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lispandfound left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

lispandfound Sep 25, 2026

Choose a reason for hiding this comment

Uh oh!

lispandfound Sep 25, 2026

Choose a reason for hiding this comment

Uh oh!

lispandfound Sep 25, 2026

Choose a reason for hiding this comment

Uh oh!

lispandfound Sep 25, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

lispandfound Sep 25, 2026

Choose a reason for hiding this comment

Uh oh!

lispandfound Sep 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

lispandfound commented Aug 4, 2026 •

edited

Loading

lispandfound Sep 25, 2026 •

edited

Loading