Update classical action of addition gates and fix classical action bug in Join #1174
Conversation
qualtran/simulation/classical_sim.py
Outdated
|
|
||
| Addition of signed integers can result in overflow. In most classical programming languages (e.g. C++) | ||
| what happens when an overflow happens is left as an implementation detail for compiler designers. | ||
| However for quantum subtraction the operation should be unitary and that means that the unitary of |
qualtran/simulation/classical_sim.py
Outdated
|
|
||
|
|
||
| def signed_addition(a: int, b: int, N: int, is_signed: bool) -> int: | ||
| """Performs addition mod N of (un)signed in a reversible way. |
There was a problem hiding this comment.
addition of signed? recommend: addition of two integers
| def test_signed_addition_signed(x, y, n_bits): | ||
| half_n = 1 << (n_bits - 1) | ||
| # Addition of signed ints `x` and `y`` is a cyclic rotation of the interval [-2^(n-1), 2^n) by `y`. | ||
| R = [*range(-(2 ** (n_bits - 1)), 2 ** (n_bits - 1))] |
There was a problem hiding this comment.
is there a good reason why we're not using pep-8 variable names here?
qualtran/simulation/classical_sim.py
Outdated
| @@ -265,3 +265,20 @@ def format_classical_truth_table( | |||
| for invals, outvals in truth_table | |||
| ] | |||
| return '\n'.join([heading] + entries) | |||
|
|
|||
|
|
|||
| def signed_addition(a: int, b: int, N: int, is_signed: bool) -> int: | |||
There was a problem hiding this comment.
Args section to document the arguments; I'd suggest renaming N to mod, and I'd suggest changing mod and is_signed to be keyword-only arguments (by putting ..., *, mod, is_signed). Baring that, I would strongly recommend using keyword args when calling this function to make the flags obvious
There was a problem hiding this comment.
done ... though I prefer to pass num_bits rather than N or mod
| @@ -95,7 +94,7 @@ def my_tensors( | |||
| ] | |||
|
|
|||
| def on_classical_vals(self, reg: 'NDArray[np.uint]') -> Dict[str, int]: | |||
| return {'reg': bits_to_ints(reg)[0]} | |||
| return {'reg': self.dtype.from_bits(reg.tolist())} | |||
There was a problem hiding this comment.
why is tolist required?
There was a problem hiding this comment.
it doesn't recognize ndarray as Sequence?
There was a problem hiding this comment.
expects Sequence[int] but got ndarray[np.int64]
|
@mpharrigan I modified my change to the classical action of Join for QFxp to retain the old behaviour ... the behaviour for QFxp will be fixed by #1142 |
No description provided.