misc: Promote to a complex double when mixing a complex float and a double - #3026
Open
MaxFreedomPollard wants to merge 1 commit into
Open
MaxFreedomPollard wants to merge 1 commit into
MaxFreedomPollard wants to merge 1 commit into
Conversation
…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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
infer_dtypeindevito/tools/dtypes_lowering.pypicks the winning dtype of an expression, and its docstring promises the floating candidate with the highest precision. It gets there withmax(fdtypes, key=lambda i: np.dtype(i).itemsize).np.complex64andnp.float64are both 8 bytes wide, so that comparison is a tie, and the tie is broken arbitrarily by set iteration order; on my machinenp.float64wins every time. An expression mixing a single precision complexFunctionwith a double one is therefore inferred to be real.extract_dtypehands that dtype on to the compiler, so a temporary introduced by CSE or CIRE to hold a complex value is declareddoubleinstead ofdouble _Complex,csindegrades tosin, and the imaginary part is discarded. The Operator builds, runs and returns wrong numbers with no warning. The Python level(f*g).dtypeis wrong too.On main:
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, andcomplex64withfloat64is the only pair whose result changes, fromfloat64tocomplex128; 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. AnEqwith a real left hand side and a mixed complex right hand side still builds, so no existing code starts raisingInvalidOperator.Verified on macOS (Apple silicon, Python 3.11, default clang,
DEVITO_LANGUAGE=C). The new testtests/test_dtypes.py::test_complex_double_promotionfails on unmodified main withAssertionError: assert <class 'numpy.float64'> is <class 'numpy.complex128'>and passes with the fix, under bothDEVITO_LANGUAGE=CandDEVITO_LANGUAGE=CXX. It checks the inferred dtype directly, checks(f*g).dtype, and comparesop.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.pyandtests/test_tools.pygive 310 passed, 3 skipped, 1 xfailed, andtests/test_derivatives.py, which reachesinfer_dtypethrough the derivatives pass, gives 376 passed. isort, ruff--preview, flake8 and typos are clean.