Conversation
|
|
||
| qlm_aqasm_keyword = "I" | ||
| qiskit_string = "id" | ||
| matrix = np.eye(2, dtype=np.complex128) |
There was a problem hiding this comment.
I think we don't do this because some of the gates' matrix can only be instantiated at __init__ time, so we try to stay homogeneous
There was a problem hiding this comment.
I see your point regarding consistency. I took a closer look at the current inheritance structure to see how we could best align these changes with the rest of the library.
From what I can see, the gates whose matrices depend on parameters (like Rx, Rz, or P) inherit from RotationGate. These don’t actually use self.matrix, but instead override to_canonical_matrix() to calculate the matrix from self.parameters.
Within NoParameterGate, the pattern seems to vary. For example, T, CNOT, and CZ also override the method directly and don't set a self.matrix attribute. It seemed that only the 8 constant gates I modified were using self.matrix in init. Since their matrices are true constants, I thought moving them might simplify the initialization.
However, I might have missed a specific convention you want to maintain here. If you feel that keeping self.matrix inside init is better for readability or consistency with future plans, I’m more than happy to revert that part!
|
In this PR, you still have 3 relatively unrelated changes, could you split them into 3 PRs ? It may sound a bit strange to have a 4 lines changed PR for your Fix 2, but for instance this one would be the one change we can accept without changing anything, while the two other modifications we will likely have to retouch them |
Fix 1 : Gate matrix class-level constants
File: native_gates.py
Promoted [self.matrix = np.array(...)] from per-instance allocation in [init] to shared class-level attributes for 8 gates: [Id], [X], [Y], [Z], [H], [S], [S_dagger], [SWAP]. All matrices now use [dtype=np.complex128] explicitly. Verified that [x1.matrix is X.matrix] instances share the class constant (zero per-instance allocation).
Fix 2 : [Instruction.eq] pickle → structural
File: instruction.py
Replaced [pickle.dumps(self) == pickle.dumps(value)] with:
Removed the from pickle import dumps import. This is now consistent with how [Gate.eq] and [Breakpoint.eq] already worked.
Fix 3 : Job unit tests
File: test_job.py
Replaced the 4-line # 3M-TODO placeholder with 276 lines covering 25 test methods across 7 test classes: [TestJobStatus], [TestJobType], [TestJobConstruction], [TestJobStatusProperty], [TestJobEquality], [TestJobRepr], and [TestGenerateJob] (including symbolic substitution).