Skip to content

Commit df6cfb9

Browse files
waltsimsclaude
andcommitted
Fix Greptile P1+P2: guard MATLAB multi-source reordering, fix FutureWarning sentinel
P1: simulate_from_dicts now reorders multi-row source signals from MATLAB's F-flat mask ordering to C-flat before passing to the solver. Previously this was documented but unguarded — callers with multi-source signals would get wrong physics. P2: FutureWarning in cart2grid, combine_sensor_data, get_distributed_source_signal now uses a sentinel default so explicit order="F" does NOT warn. Only implicit (unspecified) default triggers the deprecation warning. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent f5522e1 commit df6cfb9

5 files changed

Lines changed: 146 additions & 65 deletions

File tree

kwave/solvers/kspace_solver.py

Lines changed: 44 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -779,12 +779,52 @@ def create_simulation(kgrid, medium, source, sensor, device="auto", smooth_p0=Fa
779779
)
780780

781781

782+
def _f_to_c_source_reorder(source, grid_shape):
783+
"""Reorder multi-row source signals from MATLAB F-flat to C-flat mask order.
784+
785+
MATLAB sends source signal rows ordered by F-flattened mask indices.
786+
The solver uses C-flat ordering internally. For single-row (uniform)
787+
sources, no reordering is needed.
788+
"""
789+
ndim = len(grid_shape)
790+
if ndim < 2:
791+
return source
792+
793+
for mask_key, signal_keys in [("p_mask", ["p"]), ("u_mask", ["ux", "uy", "uz"])]:
794+
mask_raw = source.get(mask_key)
795+
if mask_raw is None:
796+
continue
797+
mask = np.asarray(mask_raw, dtype=bool)
798+
if mask.size <= 1:
799+
continue
800+
mask_grid = mask.reshape(grid_shape)
801+
n_src = int(mask_grid.sum())
802+
if n_src < 2:
803+
continue
804+
805+
# Build F→C permutation for mask points
806+
f_nz = np.where(mask_grid.ravel(order="F"))[0]
807+
c_nz = np.where(mask_grid.ravel())[0]
808+
f_equiv = np.ravel_multi_index(np.unravel_index(c_nz, grid_shape), grid_shape, order="F")
809+
perm = np.searchsorted(f_nz, f_equiv)
810+
811+
for sig_key in signal_keys:
812+
sig = source.get(sig_key)
813+
if sig is None:
814+
continue
815+
sig = np.asarray(sig)
816+
if sig.ndim >= 2 and sig.shape[0] == n_src:
817+
source[sig_key] = sig[perm]
818+
819+
return source
820+
821+
782822
def simulate_from_dicts(kgrid, medium, source, sensor, device="auto", smooth_p0=False):
783823
"""MATLAB interop entry point.
784824
785-
NOTE: The solver uses C-order internally. MATLAB sends grid-shaped arrays
786-
(which are order-agnostic via logical indexing) but source signals with
787-
rows ordered by F-flattened mask positions. Multi-row source signals from
788-
MATLAB may need reordering at the kWavePy shim layer.
825+
Reorders multi-row source signals from MATLAB's F-flat mask ordering
826+
to the solver's C-flat ordering before running the simulation.
789827
"""
828+
grid_shape = tuple(kgrid[k] for k in ["Nx", "Ny", "Nz"] if k in kgrid)
829+
source = _f_to_c_source_reorder(source, grid_shape)
790830
return create_simulation(kgrid, medium, source, sensor, device, smooth_p0=smooth_p0).run()

kwave/utils/conversion.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313
from kwave.utils.matlab import matlab_mask
1414
from kwave.utils.matrix import sort_rows
1515

16+
_default = object() # sentinel for detecting implicit default order="F"
17+
1618

1719
@typechecker
1820
def db2neper(alpha: Real[kt.ArrayLike, "..."], y: Real[kt.ScalarLike, ""] = 1) -> Real[kt.ArrayLike, "..."]:
@@ -165,7 +167,7 @@ def cart2grid(
165167
cart_data: Union[Float[ndarray, "1 NumPoints"], Float[ndarray, "2 NumPoints"], Float[ndarray, "3 NumPoints"]],
166168
axisymmetric: bool = False,
167169
*,
168-
order: str = "F",
170+
order: str = _default,
169171
) -> Tuple:
170172
"""Interpolate Cartesian points onto a binary grid using nearest neighbour.
171173
@@ -180,14 +182,15 @@ def cart2grid(
180182
Returns:
181183
(grid_data, order_index, reorder_index)
182184
"""
183-
if order == "F":
185+
if order is _default:
184186
import warnings
185187

186188
warnings.warn(
187189
"cart2grid default order='F' will change to order='C' in a future release. Pass order='C' explicitly.",
188190
FutureWarning,
189191
stacklevel=2,
190192
)
193+
order = "F"
191194

192195
# check for axisymmetric input
193196
if axisymmetric and kgrid.dim != 2:

kwave/utils/kwave_array.py

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616
from kwave.utils.math import make_affine, sinc
1717
from kwave.utils.matlab import matlab_assign, matlab_find, matlab_mask
1818

19+
_DEFAULT_ORDER = object() # sentinel for detecting implicit default order="F"
20+
1921

2022
@dataclass(eq=False)
2123
class Element:
@@ -692,15 +694,15 @@ def get_off_grid_points(self, kgrid, element_num, mask_only):
692694

693695
return grid_weights
694696

695-
def get_distributed_source_signal(self, kgrid, source_signal, *, order="F"):
697+
def get_distributed_source_signal(self, kgrid, source_signal, *, order=_DEFAULT_ORDER):
696698
"""Distribute per-element source signals onto grid source points.
697699
698700
Args:
699701
order: ``"C"`` for C-order (new API) or ``"F"`` for Fortran-order
700702
(legacy). Default ``"F"`` — will change to ``"C"`` in a
701703
future release.
702704
"""
703-
if order == "F":
705+
if order is _DEFAULT_ORDER:
704706
import warnings
705707

706708
warnings.warn(
@@ -709,6 +711,7 @@ def get_distributed_source_signal(self, kgrid, source_signal, *, order="F"):
709711
FutureWarning,
710712
stacklevel=2,
711713
)
714+
order = "F"
712715

713716
start_time = time.time()
714717
self.check_for_elements()
@@ -753,22 +756,23 @@ def get_distributed_source_signal(self, kgrid, source_signal, *, order="F"):
753756

754757
return distributed_source_signal
755758

756-
def combine_sensor_data(self, kgrid, sensor_data, *, order="F"):
759+
def combine_sensor_data(self, kgrid, sensor_data, *, order=_DEFAULT_ORDER):
757760
"""Combine sensor data from grid points back to array elements.
758761
759762
Args:
760763
order: ``"C"`` for C-order (new API) or ``"F"`` for Fortran-order
761764
(legacy). Default ``"F"`` — will change to ``"C"`` in a
762765
future release.
763766
"""
764-
if order == "F":
767+
if order is _DEFAULT_ORDER:
765768
import warnings
766769

767770
warnings.warn(
768771
"combine_sensor_data default order='F' will change to order='C' in a future release. " "Pass order='C' explicitly.",
769772
FutureWarning,
770773
stacklevel=2,
771774
)
775+
order = "F"
772776

773777
self.check_for_elements()
774778

tests/test_c_order.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,18 @@ def test_cart2grid_warns_on_f_order(self):
155155
with pytest.warns(FutureWarning, match="cart2grid"):
156156
cart2grid(kgrid, cart)
157157

158+
def test_cart2grid_no_warn_on_explicit_f(self):
159+
"""Explicit order='F' should NOT warn — only implicit default warns."""
160+
from kwave.utils.conversion import cart2grid
161+
162+
kgrid = kWaveGrid(Vector([32, 32]), Vector([1e-4, 1e-4]))
163+
cart = np.array([[0.0], [0.0]])
164+
import warnings
165+
166+
with warnings.catch_warnings():
167+
warnings.simplefilter("error", FutureWarning)
168+
cart2grid(kgrid, cart, order="F")
169+
158170
def test_cart2grid_no_warn_on_c_order(self):
159171
from kwave.utils.conversion import cart2grid
160172

0 commit comments

Comments
 (0)