[Python] Evaluate a negative literal exponent in floating point - #5168
[Python] Evaluate a negative literal exponent in floating point#5168udsy19 wants to merge 1 commit into
Conversation
In Python an integer raised to a negative integer power is a float: `2 ** -1` is `0.5`. The Python bridge lowered any integer base with an integer exponent to `math.ipowi`, an integer power whose lowering returns 0 for a negative exponent unless the base is +/-1, so `2 ** -1`, `2 ** -3` and `3 ** -1` all evaluated to 0 inside a kernel with no diagnostic. Promote the base and use `math.fpowi` when the exponent is a negative integer literal, which is the case Python types as a float. An exponent whose sign is only known at run time keeps the integer power, since the type of the expression has to be determined at compile time. Signed-off-by: Udaya Tejas <udayatejas2004@gmail.com>
I'll file an issue. It's been a while since I looked at this, but I think the conversions might be done in the C++ templates in the header files. |
|
While I sympathize that getting back a 0 isn't what a mathematician would expect here. @cudaq.kernel
def kernel(base: int, exponent: int) -> float:
return base ** exponent # kernel(2, -1) is still 0.0, not 0.5It is certainly hard to argue that the user did not specify that this was an integer It's not really a compiler's job to deliberately ignore the source code, which includes the types. Within a CUDA-Q kernel, one will not have Python's arbitrary precision arithmetic, which is definitely different than plain old Python code. As the user may be 100% cognizant that using int pow has the implementation it does and chose int types exactly for that behavior, "fixing" it so that they cannot get int pow at all seems to invite more problems than it solves. If a user wants floating point values, they need to use floating point types. |
In Python an integer raised to a negative integer power is a
float:2 ** -1is
0.5. The Python AST bridge lowered every integer base with an integerexponent to
math.ipowi, an integer power whose lowering returns 0 for anegative exponent unless the base is
+/-1, so inside a@cudaq.kernel:No error or warning was raised. Type promotion is deliberately skipped for
Pow, so nothing moved the expression into floating point.What this changes
When the exponent is a negative integer literal — the case Python types as a
float— the base is converted tof64andmath.fpowiis used instead ofmath.ipowi.math.fpowialready handles negative exponents correctly, whichis why
2.0 ** -1has always worked. Everything else is untouched:2 ** 3is still ani64math.ipowi, andrange(2 ** n)still compiles.The exponent is read from the AST rather than from the emitted SSA value,
because the bridge does not constant-fold local variables and only materialises
a constant for the literal
-1(there is an existing special case for it invisit_UnaryOp);-3is emitted asarith.muli(-1, 3). Reading the literalfrom the AST is also what the
range()step handling in this file already doeswhen it needs the sign of a literal at compile time.
__process_binary_optherefore takes the right-hand AST node as an optional argument; both call
sites (
visit_BinOp,visit_AugAssign) pass it.What this deliberately does not change
An exponent whose sign is only known at run time — a kernel parameter, or a
local such as
n = -1— keeps the integer power and the integer result type:The type of the expression has to be fixed at compile time, and Python's rule
makes it depend on the value of the exponent. Promoting
int ** intto afloat unconditionally would cover those cases, but it would change the type of
2 ** 3, breakrange(2 ** n)and integer indices like2 ** n - 1, andchange the IR checked by
python/tests/mlir/qft.py— which seems worse thanthe bug. Diagnosing it at run time would be the complete answer, but there is
no abort/assert facility in the kernel IR to build that on today. I'm happy to
follow up with whichever direction maintainers prefer; the issue lists the
options.
One visible consequence:
x **= -1on an integer variable now fails to compilewith "augment-assign must not change the variable type" instead of silently
storing
0. Python rebindsxto a float there, which a typed kernel variablecannot do, so an error is the honest outcome.
x **= -1on a float variable isunaffected (
0.5), as isx **= 3on an integer variable (8).The C++ frontend has the analogous lowering
(
cudaq/lib/Frontend/nvqpp/ConvertExpr.cpp,visitMathLibFunc): it peels offthe int-to-double conversion clang inserts for
std::pow(int, int), emitsmath.ipowi, and casts the integer result back todouble, sostd::pow(2, -1)cannot return0.5there either. Sincestd::powalwaysreturns
double, that frontend could simply usemath.fpowiunconditionally,with no type question to answer — but it is a separate change with its own
AST-Quake tests, and I could not build the C++ toolchain to validate it, so
this PR leaves it alone.
Testing
python/tests/kernel/test_kernel_float.pygains three tests:test_negative_integer_exponent_is_float— the reported cases plus anegative base and a run-time base with a literal negative exponent. Fails
before this change, passes after.
test_non_negative_integer_exponent_is_integer— guards the unchanged path:2 ** 3,2 ** 0,2 ** 30, a run-time base and exponent, andrange(2 ** 3)as a loop bound, all returningint.test_float_base_with_integer_exponent— guards the floating-point base forboth signs of the exponent.
No MLIR CHECK test changes: the new lowering only fires for a negative literal
exponent, which no existing test uses.
Fixes #5167
Validation actually performed (local, on an installed 0.15.1 wheel)
The change was applied to the
ast_bridge.pyof an installedcudaq0.15.1wheel (
cuda_quantum_cu13, Python 3.13, macOS 26.5.1 arm64), exercised end to end,and the wheel restored afterwards.
Before:
After:
New tests against the same wheel:
Wider regression subset (
test_kernel_float.py,test_assignments.py,test_cast_kernel.py,test_kernel_shift_operators.py,test_kernel_return.py,test_kernel_complex.py) run both ways against the same wheel:The 8 remaining failures are identical in both runs and are wheel-vs-
mainskew, not regressions:
test_math_*andtest_float_floor_division_errorexercise
main-only math support and error messages, andtest_assignments.py::test_var_scopeslikewise. The only difference betweenthe two runs is the new test.
yapf 0.40.2 --style google(the version pinned in.pre-commit-config.yaml)reports no diff on both changed files.
Deferred to CI (not runnable locally)
main: the C++ frontend, the lit/FileCheck suites andthe full test matrix were not run. The change cannot alter existing CHECK
output, since it only fires on a negative literal exponent and no existing
test uses one (verified by grep over
python/tests).Files changed
python/cudaq/kernel/ast_bridge.py(+32/-4) —__integerLiteralValuehelper,the
ast.Powinteger branch, and the optional right-hand AST node parameteron
__process_binary_opplus its two call sites.python/tests/kernel/test_kernel_float.py(+86) — three tests.Open questions for maintainers
(this PR), diagnose at run time, or something else?
std::pow(int, int)tomath.fpowi? Itsresult is always
double, so nothing is lost, and it would make bothfrontends agree with their own source language.