Skip to content

Python kernel % lowers to unsigned remainder, giving wrong results for negative operands #5159

Description

@udsy19

Required prerequisites

  • Consult the security policy. If reporting a security vulnerability, do not report the bug using this form. Use the process described in the policy to report the issue.
  • Make sure you've read the documentation. Your issue may be addressed there.
  • Search the issue tracker to verify that this hasn't already been reported. +1 or comment there if it has.
  • If possible, make a PR with a failing test to give us a starting point to work on!

Describe the bug

Inside a @cudaq.kernel, the % operator on signed integers lowers to an unsigned remainder, so any expression with a negative operand silently produces a wrong result. No error or warning is raised.

The Python bridge emits arith.remui:

# python/cudaq/kernel/ast_bridge.py:5951
self.pushValue(arith.RemUIOp(left, right).result)

Two things make this look like an oversight rather than an intentional choice:

  1. The C++ bridge selects a signed remainder for signed types (lib/Frontend/nvqpp/ConvertExpr.cpp:1162-1168 uses RemSIOp), so the two frontends disagree on the same source-level operator.
  2. Floor division in the same Python bridge is correctly signed (arith.floordivsi). Because // floors and % is unsigned, the identity a == (a // b) * b + a % b does not hold for negative operands.

Steps to reproduce the bug

import cudaq

@cudaq.kernel
def mod(a: int, b: int) -> int:
    return a % b

for a, b in [(-1, 3), (-5, 3), (5, -3), (7, 3)]:
    print(a, b, mod(a, b), a % b)

Output on CUDA-Q 0.15.1 (macOS arm64, cuda_quantum_cu13 wheel):

-1 3  0   2      <- wrong
-5 3  2   1      <- wrong
 5 -3 5  -1      <- wrong
 7 3  1   1      ok

Expected behavior

% inside a kernel should agree with Python semantics, as // already does — that is, the result should take the sign of the divisor, and a == (a // b) * b + a % b should hold.

Is this a regression? If it is, put the last known working version here.

Not a regression.

Environment

  • CUDA-Q version: 0.15.1 (cuda_quantum_cu13, commit aca5853)
  • Python version: 3.13
  • C++ compiler: n/a (wheel install)
  • Operating system: macOS 15 arm64

Suggested fix

Emitting arith.RemSIOp fixes the negative-dividend cases but still differs from Python when the divisor is negative, because remsi truncates toward zero. Matching Python exactly needs the floor-mod adjustment: compute r = remsi(a, b), then add b to r when r != 0 and the signs of r and b differ.

I verified the floor-mod form against the four cases above and it matches Python in all of them.

Note that PR #3513 ("Fixes for Python modulo operation") addressed the floating-point path and deliberately retained RemUIOp for integers, so this integer case is still open. The same signedness question applies to the remf path, which I have not investigated.

I'm happy to open a PR with a fix and tests if maintainers confirm the preferred semantics (full floor-mod versus simply switching to remsi).

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions