Skip to content

misc: Promote to a complex double when mixing a complex float and a double - #3026

Open
MaxFreedomPollard wants to merge 1 commit into
devitocodes:mainfrom
MaxFreedomPollard:fix/complex-dtype-promotion
Open

MaxFreedomPollard wants to merge 1 commit into
devitocodes:mainfrom
MaxFreedomPollard:fix/complex-dtype-promotion

Conversation

@MaxFreedomPollard

Copy link
Copy Markdown
Contributor

infer_dtype in devito/tools/dtypes_lowering.py picks the winning dtype of an expression, and its docstring promises the floating candidate with the highest precision. It gets there with max(fdtypes, key=lambda i: np.dtype(i).itemsize). np.complex64 and np.float64 are both 8 bytes wide, so that comparison is a tie, and the tie is broken arbitrarily by set iteration order; on my machine np.float64 wins every time. An expression mixing a single precision complex Function with a double one is therefore inferred to be real. extract_dtype hands that dtype on to the compiler, so a temporary introduced by CSE or CIRE to hold a complex value is declared double instead of double _Complex, csin degrades to sin, and the imaginary part is discarded. The Operator builds, runs and returns wrong numbers with no warning. The Python level (f*g).dtype is wrong too.

On main:

grid = Grid(shape=(4, 4))
f = Function(name='f', grid=grid, dtype=np.complex64)
g = Function(name='g', grid=grid, dtype=np.float64)
h = Function(name='h', grid=grid, dtype=np.complex128)
f.data[:] = 0.3 + 0.7j
g.data[:] = 2.0

print((f*g).dtype)  # <class 'numpy.float64'>, expected complex128

op = Operator(Eq(h, sin(f*g)*sin(f*g) + sin(f*g)))
# the generated kernel holds a complex value in a real temporary:
#     double r0 = sin(f[x + 1][y + 1]*g[x + 1][y + 1]);
op.apply()
print(h.data[0, 0])  # (0.8834636380557765+0j)
# numpy gives (0.21926883165043165+5.389281717697623j)

The fix ranks the floating candidates with np.result_type, numpy's own promotion rule, which is what the docstring already describes. I checked every pair over float16, float32, float64, complex64 and complex128, and complex64 with float64 is the only pair whose result changes, from float64 to complex128; float16 pairs, float32 with float64, complex64 with float32 and complex64 with complex128 all return exactly what they returned before. The integer branch and the single floating dtype branch are untouched, so mixed integer arithmetic keeps its current behaviour. An Eq with a real left hand side and a mixed complex right hand side still builds, so no existing code starts raising InvalidOperator.

Verified on macOS (Apple silicon, Python 3.11, default clang, DEVITO_LANGUAGE=C). The new test tests/test_dtypes.py::test_complex_double_promotion fails on unmodified main with AssertionError: assert <class 'numpy.float64'> is <class 'numpy.complex128'> and passes with the fix, under both DEVITO_LANGUAGE=C and DEVITO_LANGUAGE=CXX. It checks the inferred dtype directly, checks (f*g).dtype, and compares op.apply() against numpy; it deliberately makes no assertion about the generated source text, since that differs between the C and C++ backends. tests/test_dtypes.py, tests/test_cse.py, tests/test_symbolics.py, tests/test_differentiable.py and tests/test_tools.py give 310 passed, 3 skipped, 1 xfailed, and tests/test_derivatives.py, which reaches infer_dtype through the derivatives pass, gives 376 passed. isort, ruff --preview, flake8 and typos are clean.

…ouble

`infer_dtype` in devito/tools/dtypes_lowering.py ranked the floating
candidates by `np.dtype(i).itemsize` and returned the widest one. Since
`np.complex64` and `np.float64` are both 8 bytes wide, that comparison
was a tie, broken arbitrarily by set iteration order, and `np.float64`
won. An expression mixing a single precision complex Function with a
double one was therefore inferred to be real: compiler generated
temporaries were declared `double` instead of `double _Complex`, `csin`
degraded to `sin`, and the imaginary part was dropped at runtime with
no warning.

Ranked the candidates with `np.result_type` instead, which is numpy's
own promotion rule and already the answer the docstring describes. Among
float16, float32, float64, complex64 and complex128, `np.complex64` with
`np.float64` is the only pair whose result changes. The integer branch and the single floating
dtype branch were left alone.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant