Skip to content

Implement native gates fixes#184

Open
nm727 wants to merge 1 commit intoColibrITD-SAS:devfrom
nm727:fix-native-gates
Open

Implement native gates fixes#184
nm727 wants to merge 1 commit intoColibrITD-SAS:devfrom
nm727:fix-native-gates

Conversation

@nm727
Copy link

@nm727 nm727 commented Feb 16, 2026

No description provided.

@nm727 nm727 mentioned this pull request Feb 16, 2026
@Henri-ColibrITD
Copy link
Contributor

can you explain why these modifications are needed ? is it to save on memory ?

as mentioned before, since some functions need to be instantiated in order to initialize the matrix, so the modification you propose cannot be done for all native gates, and I'm not super convinced regarding doing the change only in the cases it doesn't pose problem.

(and the memory savings are likely negligeable?)

I'm open to be convinced otherwise :)

@nm727
Copy link
Author

nm727 commented Feb 17, 2026

@Henri-ColibrITD The motivation isn't memory, it's semantic alignment and avoiding redundant work.

Every other fixed property of these gates (qlm_aqasm_keyword, qiskit_string, braket_gate, qiskit_gate, cirq_gate) is already class-level. The matrix of X or H is just as intrinsic to the type as its qiskit_string. it felt inconsistent imo for matrix to be the odd one out rebuilt in every init call.

Moreover, the parametrized gates (Rx, Ry, P, etc.) never use self.matrix at all; they override to_canonical_matrix() with parameter-dependent logic. Same for T, CNOT, CZ, TOF. The only gates that use the matrix attribute are these 8 constant-matrix ones (Id, X, Y, Z, H, S, S†, SWAP), so the split isn't ad-hoc, it follows the existing inheritance structure.

That said, this is more of a "nice-to-have" cleanup than a critical fix. If the change seems unnecessary or less readable, I'm happy to roll it back!

If I'm missing something in my logic please do tell me! I'm open to discuss :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants