search

dwavesystems / dwave-optimization

Enables the formulation of nonlinear models for industrial optimization problems.
https://docs.ocean.dwavesys.com/en/stable/docs_optimization/index.html#index-optimization
Apache License 2.0
7 stars 18 forks source link

Consider adding a `modulus` symbol #156

Closed jlalbers closed 3 weeks ago

jlalbers commented 4 weeks ago

It would be useful to have a modulus symbol for certain scenarios (i.e. identifying weekdays over a multi-week scheduling shift). Recommend mirroring the Python implementation of the modulo operator, i.e.:

15 % 7 # -> 1
-15 % 7 # -> 6
15 % -7 # -> -6
-15 % -7 # -> -1

I can start working on a PR for this.

arcondello commented 3 weeks ago

Following NumPy on this as well seems straightforward

In [1]: arr = np.arange(5)

In [2]: arr % 2
Out[2]: array([0, 1, 0, 1, 0])

In [3]: arr % 3
Out[3]: array([0, 1, 2, 0, 1])

In [4]: 3 % arr
<ipython-input-4-369e98373d86>:1: RuntimeWarning: divide by zero encountered in remainder
  3 % arr
Out[4]: array([0, 0, 1, 0, 3])

although the divide-by-zero case worries me. It looks like they just assign a % 0 to 0. We should be careful to also handle that case. As well as non-integer modulus operations.

jlalbers commented 3 weeks ago

CPython has a modulo implementation for floats, so we can use that as a reference. Will check the numpy source as well to see how they're doing it. I imagine we could handle a mix of bool/integer/float values via widening conversions.

For the divide-by-zero case, I think it would be good to copy numpy and allow the operation while emitting a warning. This would prevent perceived "random" failures and at least provide a defined behaviour. I think it's also consistent with the definition of modulus as a mathematical concept (as opposed to a remainder).