-
Notifications
You must be signed in to change notification settings - Fork 54
Fix alpha mode near unity #704
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from 6 commits
8422347
1dfb845
c5b35b7
ba1adea
966647a
6e1e5ef
7a4ce97
2f985fa
bcef90d
73b5584
fb0c0e3
ccce919
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,94 @@ | ||
| # CLAUDE.md | ||
|
|
||
| This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository. | ||
|
|
||
| ## What is k-wave-python? | ||
|
|
||
| Python implementation of the [k-Wave](http://www.k-wave.org/) acoustics toolbox for time-domain acoustic and ultrasound simulations. Two backends: a pure Python/NumPy/CuPy solver and C++ binaries (OMP/CUDA). | ||
|
|
||
| ## Commands | ||
|
|
||
| ```bash | ||
| # Setup | ||
| uv sync --extra dev --extra test | ||
| uv run pre-commit install | ||
|
|
||
| # Run tests | ||
| uv run pytest # all tests | ||
| uv run pytest tests/test_kgrid.py # single file | ||
| uv run pytest tests/test_kgrid.py::test_name # single test | ||
| uv run pytest -m integration # MATLAB-reference tests only | ||
|
|
||
| # Lint/format | ||
| uv run ruff check . --fix | ||
| uv run ruff format . | ||
| uv run pre-commit run --all-files | ||
|
|
||
| # Run an example | ||
| uv run examples/ivp_homogeneous_medium.py | ||
|
|
||
| # Build | ||
| uv build | ||
| ``` | ||
|
|
||
| Always use `uv run` (not `uv run python`) for pytest, examples, and scripts. | ||
|
|
||
| ## Code Style | ||
|
|
||
| - Line length: 140 (configured in `pyproject.toml` under `[tool.ruff]`) | ||
| - Pre-commit hooks: ruff (lint + format), codespell, nb-clean | ||
| - Ruff ignores F821 (nested function refs) and F722 (jaxtyping annotations) | ||
| - Type annotations use `beartype` + `jaxtyping` | ||
|
|
||
| ## Architecture | ||
|
|
||
| ### Entry point | ||
|
|
||
| `kwave/kspaceFirstOrder.py` — `kspaceFirstOrder()` is the unified API. It accepts `kgrid`, `medium`, `source`, `sensor` and dispatches to the appropriate backend. | ||
|
|
||
| ``` | ||
| kspaceFirstOrder(kgrid, medium, source, sensor, backend="python"|"cpp", device="cpu"|"gpu") | ||
| ``` | ||
|
|
||
| ### Two backends | ||
|
|
||
| - **Python backend** (`kwave/solvers/kspace_solver.py`): `Simulation` class implementing k-space pseudospectral method in NumPy/CuPy. Supports 1D/2D/3D. Uses CuPy for GPU when `device="gpu"`. | ||
| - **C++ backend** (`kwave/solvers/cpp_simulation.py` + `kwave/executor.py`): Serializes to HDF5, invokes compiled C++ binary, reads results back. `save_only=True` writes HDF5 without running (for cluster jobs). | ||
|
|
||
| ### Core data classes | ||
|
|
||
| - `kWaveGrid` (`kwave/kgrid.py`) — domain discretization, spacing, time array | ||
| - `kWaveMedium` (`kwave/kmedium.py`) — sound speed, density, absorption, nonlinearity | ||
| - `kSource` (`kwave/ksource.py`) — pressure/velocity sources with masks and signals | ||
| - `kSensor` (`kwave/ksensor.py`) — sensor mask and recording configuration | ||
|
|
||
| ### Legacy path | ||
|
|
||
| `kspaceFirstOrder2D()` / `kspaceFirstOrder3D()` in their respective files route through `kWaveSimulation` (`kwave/kWaveSimulation.py`) — a large legacy dataclass used by the C++ backend path. New code should use `kspaceFirstOrder()` directly. | ||
|
|
||
| ### PML handling | ||
|
|
||
| When `pml_inside=False` (default), `kspaceFirstOrder()` expands the grid by `2*pml_size` before simulation and strips PML from full-grid output fields afterward. `pml_size="auto"` selects optimal sizes via `get_optimal_pml_size()`. | ||
|
|
||
| ### Key utilities | ||
|
|
||
| - `kwave/utils/pml.py` — PML sizing (`get_pml()`, `get_optimal_pml_size()`) | ||
| - `kwave/utils/mapgen.py` — geometry generators (`make_disc`, `make_ball`, `make_cart_circle`, etc.) | ||
| - `kwave/utils/signals.py` — signal generation (tone bursts, filtering) | ||
| - `kwave/utils/filters.py` — spatial smoothing, Gaussian filters | ||
| - `kwave/utils/io.py` — HDF5 read/write | ||
| - `kwave/utils/conversion.py` — unit conversion, `cart2grid` | ||
|
|
||
| ## Testing | ||
|
|
||
| - Tests in `tests/`, configured via `[tool.pytest.ini_options]` in `pyproject.toml` | ||
| - Integration tests (`@pytest.mark.integration`) compare against MATLAB reference data | ||
| - Test fixtures in `tests/integration/conftest.py`: `load_matlab_ref`, `assert_fields_close` | ||
| - MATLAB reference data pointed to by `KWAVE_MATLAB_REF_DIR` env var | ||
|
|
||
| ## Naming Conventions | ||
|
|
||
| - `backend="python"` or `"cpp"` (not "native") | ||
| - `device="cpu"` or `"gpu"` (not `use_gpu` bool) | ||
| - `quiet=True` to suppress output; `debug=True` for detailed output | ||
| - `pml_size="auto"` for automatic PML sizing (not a separate boolean) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -47,6 +47,14 @@ def validate_medium(medium, kgrid): | |
| power = float(np.asarray(medium.alpha_power).flat[0]) | ||
| if power < 0 or power > 3: | ||
| warnings.warn(f"medium.alpha_power={power} is outside typical range [0, 3].", stacklevel=3) | ||
| alpha_mode = getattr(medium, "alpha_mode", None) | ||
| if abs(power - 1.0) < 0.05 and alpha_mode != "no_dispersion": | ||
| raise ValueError( | ||
| f"medium.alpha_power={power} is too close to 1.0. The dispersion term " | ||
| f"contains tan(pi*alpha_power/2), which diverges at alpha_power=1. " | ||
| f"Set medium.alpha_mode='no_dispersion' to disable the dispersion term, " | ||
| f"or use an alpha_power value further from 1.0." | ||
| ) | ||
|
Comment on lines
+51
to
+58
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The guard at line 51 raises a The problem is the suggested escape hatch —
The error message even recommends Consider either (a) skipping this check when the C++ backend is active (threading the if abs(power - 1.0) < 0.05 and alpha_mode != "no_dispersion":
raise ValueError(
f"medium.alpha_power={power} is too close to 1.0. The dispersion term "
f"contains tan(pi*alpha_power/2), which diverges at alpha_power=1. "
f"For backend='python': set medium.alpha_mode='no_dispersion' to disable "
f"the dispersion term. For backend='cpp' or to use both absorption and "
f"dispersion, choose an alpha_power value further from 1.0."
) |
||
|
|
||
|
|
||
| def validate_pml(pml_size, kgrid): | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,132 @@ | ||
| """Tests for alpha_power near 1.0 (tan(pi*y/2) singularity).""" | ||
| import numpy as np | ||
| import pytest | ||
|
|
||
| from kwave.kgrid import kWaveGrid | ||
| from kwave.kmedium import kWaveMedium | ||
| from kwave.ksensor import kSensor | ||
| from kwave.ksource import kSource | ||
| from kwave.kspaceFirstOrder import kspaceFirstOrder | ||
|
|
||
|
|
||
| def _make_sim(): | ||
| """Create a minimal 2D simulation with a point source.""" | ||
| N = (64, 64) | ||
| dx = (1e-4, 1e-4) | ||
| kgrid = kWaveGrid(N, dx) | ||
| kgrid.makeTime(1500) | ||
|
|
||
| source = kSource() | ||
| source.p0 = np.zeros(N) | ||
| source.p0[32, 32] = 1.0 | ||
|
|
||
| sensor = kSensor(mask=np.ones(N)) | ||
| return kgrid, source, sensor | ||
|
|
||
|
|
||
| def test_alpha_power_near_unity_raises_without_no_dispersion(): | ||
| """alpha_power close to 1.0 must raise when dispersion is enabled.""" | ||
| kgrid, source, sensor = _make_sim() | ||
| medium = kWaveMedium(sound_speed=1500, density=1000, alpha_coeff=0.5, alpha_power=1.01) | ||
|
|
||
| with pytest.raises(ValueError, match="too close to 1.0"): | ||
| kspaceFirstOrder(kgrid, medium, source, sensor, pml_inside=True, quiet=True) | ||
|
|
||
|
|
||
| def test_alpha_power_near_unity_works_with_no_dispersion(): | ||
| """alpha_power close to 1.0 must produce valid output when dispersion is disabled.""" | ||
| kgrid, source, sensor = _make_sim() | ||
| medium = kWaveMedium( | ||
| sound_speed=1500, | ||
| density=1000, | ||
| alpha_coeff=0.5, | ||
| alpha_power=1.01, | ||
| alpha_mode="no_dispersion", | ||
| ) | ||
|
|
||
| result = kspaceFirstOrder(kgrid, medium, source, sensor, pml_inside=True, quiet=True) | ||
| p = np.asarray(result["p"]) | ||
| assert not np.any(np.isnan(p)), "Output contains NaN with alpha_power=1.01 and no_dispersion" | ||
| assert not np.all(p == 0), "Output is all zeros — absorption had no effect" | ||
|
|
||
|
|
||
| def test_alpha_mode_no_absorption(): | ||
| """alpha_mode='no_absorption' should disable absorption but keep dispersion.""" | ||
| kgrid, source, sensor = _make_sim() | ||
| medium_lossy = kWaveMedium(sound_speed=1500, density=1000, alpha_coeff=0.5, alpha_power=1.5) | ||
| medium_no_abs = kWaveMedium( | ||
| sound_speed=1500, | ||
| density=1000, | ||
| alpha_coeff=0.5, | ||
| alpha_power=1.5, | ||
| alpha_mode="no_absorption", | ||
| ) | ||
|
|
||
| result_lossy = kspaceFirstOrder(kgrid, medium_lossy, source, sensor, pml_inside=True, quiet=True) | ||
| result_no_abs = kspaceFirstOrder(kgrid, medium_no_abs, source, sensor, pml_inside=True, quiet=True) | ||
|
|
||
| p_lossy = np.asarray(result_lossy["p"]) | ||
| p_no_abs = np.asarray(result_no_abs["p"]) | ||
|
|
||
| assert not np.any(np.isnan(p_no_abs)) | ||
| # Disabling absorption should change the output | ||
| assert not np.allclose(p_lossy, p_no_abs), "Disabling absorption should change the output" | ||
|
|
||
|
|
||
| def test_alpha_mode_no_dispersion(): | ||
| """alpha_mode='no_dispersion' should disable dispersion but keep absorption.""" | ||
| kgrid, source, sensor = _make_sim() | ||
| medium_lossy = kWaveMedium(sound_speed=1500, density=1000, alpha_coeff=0.5, alpha_power=1.5) | ||
| medium_no_disp = kWaveMedium( | ||
| sound_speed=1500, | ||
| density=1000, | ||
| alpha_coeff=0.5, | ||
| alpha_power=1.5, | ||
| alpha_mode="no_dispersion", | ||
| ) | ||
|
|
||
| result_lossy = kspaceFirstOrder(kgrid, medium_lossy, source, sensor, pml_inside=True, quiet=True) | ||
| result_no_disp = kspaceFirstOrder(kgrid, medium_no_disp, source, sensor, pml_inside=True, quiet=True) | ||
|
|
||
| p_lossy = np.asarray(result_lossy["p"]) | ||
| p_no_disp = np.asarray(result_no_disp["p"]) | ||
|
|
||
| assert not np.any(np.isnan(p_no_disp)) | ||
| # Both should have absorption so similar amplitude, but waveforms differ | ||
| assert not np.allclose(p_lossy, p_no_disp), "Disabling dispersion should change the output" | ||
|
|
||
|
|
||
| def test_alpha_power_normal_range_unaffected(): | ||
| """alpha_power=1.5 (well away from singularity) should work as before.""" | ||
| kgrid, source, sensor = _make_sim() | ||
| medium = kWaveMedium(sound_speed=1500, density=1000, alpha_coeff=0.5, alpha_power=1.5) | ||
|
|
||
| result = kspaceFirstOrder(kgrid, medium, source, sensor, pml_inside=True, quiet=True) | ||
| p = np.asarray(result["p"]) | ||
| assert not np.any(np.isnan(p)) | ||
| assert p.shape[0] > 0 | ||
|
|
||
|
|
||
| def test_cpp_backend_warns_on_alpha_mode(tmp_path): | ||
| """C++ backend should warn when alpha_mode is set (it cannot honor it).""" | ||
| kgrid, source, sensor = _make_sim() | ||
| medium = kWaveMedium( | ||
| sound_speed=1500, | ||
| density=1000, | ||
| alpha_coeff=0.5, | ||
| alpha_power=1.5, | ||
| alpha_mode="no_dispersion", | ||
| ) | ||
|
|
||
| with pytest.warns(UserWarning, match="not supported by the C\\+\\+ backend"): | ||
| kspaceFirstOrder( | ||
| kgrid, | ||
| medium, | ||
| source, | ||
| sensor, | ||
| pml_inside=True, | ||
| quiet=True, | ||
| backend="cpp", | ||
| save_only=True, | ||
| data_path=str(tmp_path), | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nabla1computed unnecessarily when absorption is disablednabla1(a fractional Laplacian, potentially expensive) is computed unconditionally, but whenno_absorption=Truethe resulting lambda always returns0andnabla1is never used. Movingnabla1inside the else branch avoids the wasted computation.