Port examples to new api by waltsims · Pull Request #668 · waltsims/k-wave-python

waltsims · 2026-03-23T03:38:54Z

Greptile Summary

This PR migrates ~30 examples to the new unified kspaceFirstOrder() API, adds CI infrastructure for running examples and auto-generating notebooks, and cleans up the old per-dimensionality solver calls. The overall direction is a clear improvement in usability and consistency.

Key changes:

All example scripts moved to examples/*.py (flat) and updated to call kspaceFirstOrder() with backend=/device= kwargs instead of separate kspaceFirstOrder2D / 3D functions.
kspaceFirstOrder() gains @typechecker, env-var overrides (KWAVE_BACKEND, KWAVE_DEVICE), a pml_inside=True warning, and Union[kSource, NotATransducer] on the source parameter.
validation.py correctly switches all direct attribute accesses on source to getattr(...) guards so NotATransducer objects pass validation cleanly.

Issues found in this review:

Critical (P0): source.p0 is not None on line 199 of kspaceFirstOrder.py raises AttributeError for every caller passing a NotATransducer as source; fix is getattr(source, "p0", None).
Warning (P1): run-examples.yml omits KWAVE_BACKEND: python, so examples hardcoding backend="cpp" will fail in CI.

Confidence Score: 2/5

Not safe to merge — guaranteed AttributeError crash for all NotATransducer-based simulations.

P0 bug causes runtime crash in transducer examples; several prior-thread issues remain open.

kwave/kspaceFirstOrder.py line 199, .github/workflows/run-examples.yml

Important Files Changed

Filename	Overview
kwave/kspaceFirstOrder.py	Unified entry point; adds typechecker and NotATransducer support but source.p0 access crashes for NotATransducer sources.
kwave/solvers/validation.py	All source attribute accesses replaced with getattr guards; correct improvement.
.github/workflows/run-examples.yml	Missing KWAVE_BACKEND=python env var causes C++-backend examples to fail in CI.
run_examples.py	Rewritten with proper env vars and failure reporting; solid improvement.
tests/test_kspaceFirstOrder.py	New tests for env-var overrides and Cartesian sensor conversion are well-targeted.

Comments Outside Diff (4)

kwave/kspaceFirstOrder.py, line 143-152 (link)

sensor.record_start_index silently ignored for cpp backend — steady-state results will be wrong

Several examples (e.g. at_circular_piston_3D, at_focused_bowl_3D, at_circular_piston_AS, at_focused_bowl_AS, at_focused_annular_array_3D) set sensor.record_start_index = kgrid.Nt - (record_periods * ppp) + 1 specifically to capture only the final steady-state window. When the cpp backend ignores this and records from time step 1, extract_amp_phase is called on all time steps — the early transient contaminates the result and the extracted amplitude will be wrong.

The warning on line 147 is not enough; the simulation produces incorrect output without user awareness. Options:
- Raise a ValueError instead of a warning when record_start_index != 1 and backend="cpp" (breaking but safe).
- Pass the value to the C++ binary's dataset sensor_record_start_index if the binary supports it.
- Add a note in the warning explicitly saying the output will differ from the intended steady-state window.
kwave/kspaceFirstOrder.py, line 206 (link)

AttributeError when source is NotATransducer

source.p0 is accessed directly, but NotATransducer (which inherits from kSensor) has no p0 attribute. Since kSensor.__init__ never sets p0 and there is no __getattr__ fallback, this raises AttributeError at runtime whenever smooth_p0=True (the default) and a transducer is passed as source — for example in us_bmode_phased_array.py.

The validation layer was correctly updated to use getattr(source, "p0", None), but the guard here was not:
kwave/kspaceFirstOrder.py, line 200 (link)

AttributeError for NotATransducer source when smooth_p0=True

NotATransducer is a subclass of kSensor and has no p0 attribute. The direct attribute access source.p0 here will raise AttributeError: 'NotATransducer' object has no attribute 'p0' whenever a transducer is passed as source with the default smooth_p0=True.

us_bmode_phased_array.py passes not_transducer (a NotATransducer) as source without overriding smooth_p0, so this code path is hit and will fail in CI.

validation.py already uses getattr(source, "p0", None) to guard against exactly this case — the same pattern should be applied here:
kwave/kspaceFirstOrder.py, line 199 (link)

AttributeError when source is NotATransducer

NotATransducer inherits from kSensor, not kSource, and defines no p0 attribute. With smooth_p0=True (the default), Python evaluates source.p0 as soon as the expression is reached and raises AttributeError for every caller that passes a transducer as source (e.g. us_bmode_phased_array.py, us_bmode_linear_transducer.py).

_{Reviews (5): Last reviewed commit: "CI: run all examples with native backend..." | Re-trigger Greptile}

Greptile also left 1 inline comment on this PR.

…cpp backend - @typechecker on kspaceFirstOrder() catches wrong positional arg order - Auto-convert Cartesian sensor masks to binary via cart2grid for cpp backend (python backend handles Cartesian masks natively via interpolation) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

- Replace kspaceFirstOrder2DC/3D with kspaceFirstOrder() - Remove SimulationOptions/SimulationExecutionOptions usage - Map is_gpu_simulation → device="gpu"|"cpu", save_to_disk → implicit for backend="cpp" - Drop data_cast="single" (handled internally) - Note unsupported features: pml_inside, stream_to_disk, checkpointing - Time-reversal examples keep legacy imports (TimeReversal class requires them) - checkpointing example unchanged (uses checkpoint_file/checkpoint_timesteps not yet in new API) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

The new kspaceFirstOrder() API does not mutate its inputs (handles copies internally when needed), so deepcopy is never required. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

All .py examples now work as interactive notebooks in VS Code/JupyterLab via percent-format cell markers. Each file has a markdown header cell with title and description, followed by logical sections. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

…es README .py files with # %% markers are the source of truth. Notebooks will be auto-generated via jupytext at release time. Per-example READMEs replaced by inline markdown headers in each .py file. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

jupytext is a dev/CI dependency only (not runtime). Notebooks are generated from .py files with: jupytext --to notebook examples/**/*.py Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

…es on CPU/python kspaceFirstOrder() now respects KWAVE_BACKEND and KWAVE_DEVICE env vars, allowing CI to run GPU/cpp examples with python/cpu backend. CI workflow sets these + MPLBACKEND=Agg, and excludes utility/legacy-only scripts. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

codecov · 2026-03-23T03:39:15Z

Codecov Report

❌ Patch coverage is 92.85714% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 74.17%. Comparing base (502565a) to head (74a7728).

Files with missing lines	Patch %	Lines
kwave/solvers/validation.py	80.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #668      +/-   ##
==========================================
+ Coverage   74.09%   74.17%   +0.07%     
==========================================
  Files          56       56              
  Lines        7925     7933       +8     
  Branches     1543     1544       +1     
==========================================
+ Hits         5872     5884      +12     
+ Misses       1435     1432       -3     
+ Partials      618      617       -1

Flag	Coverage Δ
3.10	`74.17% <92.85%> (+0.07%)`	⬆️
3.11	`74.17% <92.85%> (+0.07%)`	⬆️
3.12	`74.17% <92.85%> (+0.07%)`	⬆️
3.13	`74.17% <92.85%> (+0.07%)`	⬆️
macos-latest	`74.14% <92.85%> (+0.07%)`	⬆️
ubuntu-latest	`74.14% <92.85%> (+0.07%)`	⬆️
windows-latest	`74.15% <92.85%> (+0.07%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Covers KWAVE_BACKEND/KWAVE_DEVICE env override paths and cart2grid auto-conversion for cpp backend. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

kwave/kspaceFirstOrder.py

examples/pr_2D_TR_line_sensor/pr_2D_TR_line_sensor.py

NotATransducer (kSensor subclass) is used as source in transducer examples (us_bmode_phased_array). beartype rejects it without the Union[kSource, NotATransducer] annotation. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

…level - Revert at_focused_bowl_AS and at_circular_piston_AS to kspaceFirstOrderASC (new API doesn't support axisymmetric flag — would silently produce wrong results) - Hoist legacy imports (kspaceFirstOrder2D/3D, SimulationOptions, etc.) to top of TR examples instead of burying inside main() Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

…badge On merge to master, CI generates .ipynb files from examples/*.py via jupytext and commits them to notebooks/. Colab badges link to these generated notebooks. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

waltsims · 2026-03-23T03:54:29Z

@greptile-app

- run_examples.py sets KWAVE_BACKEND=python KWAVE_DEVICE=cpu MPLBACKEND=Agg - Skip utility files, report pass/fail summary - Add notebooks/.gitkeep so link checker doesn't fail on missing directory Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

examples/at_circular_piston_AS.py

examples/at_focused_bowl_AS.py

examples/pr_2D_TR_line_sensor.py

.github/workflows/generate-notebooks.yml

…ectories - Move 20 example .py files from subdirectories to examples/ - Inline example_utils.py into us_bmode_linear_transducer.py - Delete now-empty subdirectories and stale MATLAB file - README.md renders below file list on GitHub (always visible) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

… workflows - Fix: import kspaceFirstOrderASC from kwave.kspaceFirstOrderAS (not 2D) - generate-notebooks: flat glob, explicit push to master, contents:write permission - run-examples: exclude AS examples (legacy API), flat find with maxdepth 1 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

- Squeeze single-sensor data before plotting (ivp_photoacoustic_waveforms) - Fix sensor sum axis: axis=0 sums across sensors for (n_sensors, n_time) shape (sd_focussed_detector_2D, sd_focussed_detector_3D, sd_directivity_modelling_2D) - Add missing kgrid.makeTime() call (pr_3D_FFT_planar_sensor) - Use full grid size (PML inside) instead of subtracting PML (pr_3D_FFT_planar_sensor) - Skip save_only test when KWAVE_BACKEND=python (unified_entry_point_demo) 11/20 examples now pass with KWAVE_BACKEND=python. Remaining failures are in kWaveArray/transducer examples that need library-level fixes. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

…ansducer - Remove .T on sensor_data["p"] — python backend already returns (n_sensors, n_time) (at_array_as_sensor, at_circular_piston_3D, at_focused_bowl_3D, at_focused_annular_array_3D, pr_3D_FFT_planar_sensor) - Fix PML inside: use full grid size, don't subtract 2*PML (pr_2D_FFT_line_sensor, at_circular_piston_3D) - validate_source: use getattr() for all source attributes so NotATransducer (which lacks p0, p, p_mask, u_mask) doesn't raise AttributeError 17/20 examples now pass with KWAVE_BACKEND=python. Remaining: - us_* (3): still on legacy API, need NotATransducer python backend support - at_array_as_source: ffmpeg/mp4 dependency Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

- CLAUDE.md: project overview, architecture, conventions, commands - release-strategy.md: detailed F-order → C-order migration plan for v1.0 with explicit boundary layers (HDF5 serialization, MATLAB interop) - Document 1-based → 0-based indexing migration path Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

…nside=False When sensor=None with pml_inside=False, the grid expands (64 → 104) and all grid points become sensors. _strip_pml now strips PML rows from time-series data using an interior boolean mask on the expanded grid. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

waltsims · 2026-03-23T15:41:45Z

@greptile-app

greptile-apps · 2026-03-23T15:46:43Z

kwave/kspaceFirstOrder.py

 from typing import Optional, Union

 import numpy as np
+from beartype import beartype as typechecker


typechecker imported but never applied

beartype as typechecker is imported but no function in this file is decorated with @typechecker. This leaves an unused import that linters will flag and could confuse readers who expect runtime type-checking to be active.

Either apply it to kspaceFirstOrder (which would enforce the Union[kSource, NotATransducer] annotation at call-time), or remove the import if it is not needed here:

Suggested change

from beartype import beartype as typechecker

(i.e. remove this line entirely, or add @typechecker before def kspaceFirstOrder(...) if enforcement is intended.)

kwave/kspaceFirstOrder.py

greptile-apps · 2026-03-23T15:46:46Z

run_examples.py

+import sys
 from pathlib import Path

+SKIP = {"example_utils.py", "__init__.py"}


checkpoint.py not excluded from run_examples.py

The CI workflow (run-examples.yml) explicitly skips checkpoint.py with -not -name "checkpoint.py", but run_examples.py only skips example_utils.py and __init__.py. Because rglob("*.py") recurses into examples/checkpointing/, checkpoint.py will be executed when anyone runs python run_examples.py locally. If that script requires special conditions (e.g. a C++ binary, specific file paths, or manual cleanup), it will fail or leave artifacts.

Suggested change

SKIP = {"example_utils.py", "__init__.py"}

SKIP = {"example_utils.py", "__init__.py", "checkpoint.py"}

…store @typechecker Post-merge cleanup: - Remove _is_binary_mask (superseded by _is_cartesian_mask from pml-inside branch) - Remove sensor_was_none (assigned but never read after merge resolution) - Restore @typechecker decorator on kspaceFirstOrder (catches wrong arg order) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

waltsims · 2026-03-23T15:58:16Z

@greptile-app

kwave/kspaceFirstOrder.py

Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>

Remove KWAVE_BACKEND override — examples run with their specified backend (cpp or python). Only override KWAVE_DEVICE=cpu since CI has no GPU. Include all examples including legacy ones (TR, AS, checkpoint). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

greptile-apps · 2026-03-26T15:46:23Z

.github/workflows/run-examples.yml

      - name: Run example
        env:
-          KWAVE_FORCE_CPU: 1
+          KWAVE_DEVICE: cpu
+          MPLBACKEND: Agg


KWAVE_BACKEND not set; C++-backend examples will fail in CI

KWAVE_DEVICE: cpu is set, but KWAVE_BACKEND: python is missing. us_bmode_phased_array.py hardcodes backend="cpp" and is no longer excluded from CI discovery.

Suggested change

- name: Run example

env:

KWAVE_FORCE_CPU: 1

KWAVE_DEVICE: cpu

MPLBACKEND: Agg

env:

KWAVE_BACKEND: python

KWAVE_DEVICE: cpu

MPLBACKEND: Agg

waltsims and others added 7 commits March 22, 2026 20:11

Remove unnecessary deepcopy from all examples

191a849

The new kspaceFirstOrder() API does not mutate its inputs (handles copies internally when needed), so deepcopy is never required. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Add jupytext config and dev dependency for notebook generation

98e092b

jupytext is a dev/CI dependency only (not runtime). Notebooks are generated from .py files with: jupytext --to notebook examples/**/*.py Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Add tests for env var overrides and Cartesian sensor mask conversion

119dd32

Covers KWAVE_BACKEND/KWAVE_DEVICE env override paths and cart2grid auto-conversion for cpp backend. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

greptile-apps bot reviewed Mar 23, 2026

View reviewed changes

kwave/kspaceFirstOrder.py Show resolved Hide resolved

kwave/kspaceFirstOrder.py Outdated Show resolved Hide resolved

examples/pr_2D_TR_line_sensor/pr_2D_TR_line_sensor.py Outdated Show resolved Hide resolved

waltsims and others added 3 commits March 22, 2026 20:45

greptile-apps bot reviewed Mar 23, 2026

View reviewed changes

examples/at_circular_piston_AS.py Show resolved Hide resolved

examples/at_focused_bowl_AS.py Show resolved Hide resolved

examples/pr_2D_TR_line_sensor.py Show resolved Hide resolved

.github/workflows/generate-notebooks.yml Show resolved Hide resolved

waltsims and others added 7 commits March 22, 2026 20:59

Merge branch 'master' into port-examples-to-new-api

0b8fd66

greptile-apps bot reviewed Mar 23, 2026

View reviewed changes

kwave/kspaceFirstOrder.py Outdated Show resolved Hide resolved

waltsims and others added 2 commits March 23, 2026 09:15

Update kspaceFirstOrder.py remobe unused local var

bbbd4dd

Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>

greptile-apps bot reviewed Mar 26, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Port examples to new api#668

Port examples to new api#668
waltsims wants to merge 22 commits intomasterfrom
port-examples-to-new-api

waltsims commented Mar 23, 2026 •

edited by greptile-apps bot

Loading

Uh oh!

codecov bot commented Mar 23, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

waltsims commented Mar 23, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

waltsims commented Mar 23, 2026

Uh oh!

greptile-apps bot Mar 23, 2026

Uh oh!

Uh oh!

Uh oh!

greptile-apps bot Mar 23, 2026

Uh oh!

waltsims commented Mar 23, 2026

Uh oh!

Uh oh!

greptile-apps bot Mar 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

	SKIP = {"example_utils.py", "__init__.py"}
	SKIP = {"example_utils.py", "__init__.py", "checkpoint.py"}

Conversation

waltsims commented Mar 23, 2026 • edited by greptile-apps bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 2/5

Important Files Changed

Comments Outside Diff (4)

Uh oh!

codecov bot commented Mar 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

waltsims commented Mar 23, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

waltsims commented Mar 23, 2026

Uh oh!

greptile-apps bot Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

greptile-apps bot Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

waltsims commented Mar 23, 2026

Uh oh!

Uh oh!

greptile-apps bot Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

waltsims commented Mar 23, 2026 •

edited by greptile-apps bot

Loading

codecov bot commented Mar 23, 2026 •

edited

Loading