Conversation
Removed unused variables and minor corrections in zzcrosstalk
…instantiation across documentation
…ontrolled Gate instead
**Changes:** - Depreciated N in `QubitCircuit` and replaced it with `num_qubits` instead. - Replaced `qc.N` with `qc.num_qubits` in the codebase - Depreciated `N` in Gatecompiler. - Replaced N with `num_qubits` in TexRenderer (N was not an input argument btw)
2f07c0d to
dde9bac
Compare
|
Glad to see this coming! It seems like a very big collection. Are there ways to make it smaller? Maybe only the parts that resolve the master branch conflict? |
|
Hi, Yes, the number of commits did considerably exceed my expectation for this PR. And there is still a little work left. I can split this PR into two (one before merge master) and one after, this should help with reviewing. Any changes as per the code review for the first half, I will do in the second PR to avoid any merge conflicts. How does it sound? Also the major code changes are operations/, circuit.py and test_gates.py, it's better to review those files first. |
Yes, that sounds better. The merging one will require less code review. |
dde9bac to
8f6bd83
Compare
BoxiLi
left a comment
There was a problem hiding this comment.
I was wondering about the name std can we find something more user-friendly? Or simply encourage people to import from the gateclass module.
from qutip.operations import gateclass
gateclass.X
Though I still think it would be convenient to keep the string input.
BoxiLi
left a comment
There was a problem hiding this comment.
Another thing I find slightly inconsistent is parameteric gates and std gates. For a parameteric gate, one calls parametricgate(argvalue).get_qobj(), for other gates like std, it is simply stdgate.get_qobj. A static method. I was wondering if it will cause confusion.
Ok there is a deeper reason for it. A Parametric Gate needs to be initialised with the parameter RX(0.5) only then can any gate method on it be called e.g. Also static method does mean a property so it is |
Yes, we can always rename
I should have explained why I am in the favour of deprecating string input in the PR description. First this create inconsistency, "X" is fine but something like "RX" requires an Second thing, I agree string inputs are convenient. But now the user can do something like so his string inputs only change from "gate" to Third thing, in the second PR I introduce the concept of namespaces. So each gate must belong to a namespace (default is std, not to be confused with the dir |
This might work, but let's make it in a separate PR at the end.
Ok, I am convinced. |
Backward compatibility and the deprecation warning are already there in this PR. |
|
Ok great, I missed this. If |
|
@BoxiLi you can go ahead with the merge |
The original PR #325 was split into 2, for the previous set of changes look at that PR description. - Defined `__slots__` for all the Abstract Gate classes and standard gate classes for better memory efficiency and faster lookup time. - Added boolean `is_controlled`, `is_parametric` methods to the `Gate` class. - Added the concept and implementation of namespaces for operations. This is done because at several places earlier gate.name was being used for comparison. Each operation (gate for now) must now be defined in a namespace or if it is a temporary gate, namespace can be set to None. This allows the user to define a faulty version of standard gates or follow their own convention for certain Parametric gates. The concept of namespace is much broader. Currently the parent namespace is "std", under it only "gates" is set as of now. In future we can make other namespaces under "std" like for Arithmetic Ops, Ansatz Ops etc. For the idea of Ops look at #263 (comment) Though it hasn't been implemented yet. In a way std namespace will work like a standard library for different operations and the user can also define their own implementation for operations in a different namespace. - Added caching in `get_qobj` method for standard gates. This is done for two reasons. ``` @staticmethod def get_qobj() -> Qobj: return Qobj([[1, 0], [0, -1]]) ``` If you run it multiple times and check `id()` for each obj, they will be different. This leads to higher memory consumption for large circuit simulations, even though all of them represent the Z gate's qobj in this case. Additonally different qobjs will each need to be garbage collected. Memory and lookup are an even larger bottleneck in VQA circuits where several times many gates share the same parameter. - Added `dtype` to `get_qobj(dtype="dense")` method, . This might be useful if the circuit simulation is run on a different backend like JAX, CUDA. - Made `Gate` non initialised by default i.e. `__init__` raise will raise an Error. Obviously`ParametricGate` and its controlled versions are initialisable. This makes sense because target, controls have been removed from gates and `x1=X()`, `x2=X()` now represent the same thing. Also good from a memory point of view. - Added `Sdag`, `Tdag`, `SQRTXdag`, `ISWAPdag`, `SQRTSWAPdag`, `BERKELEYdag` to the standard gates. They have been added to the documentation table and in the circuit draw (colour config for them). - Added `inverse` method for `Gate` class. Also implemented inverse for all standard gates and tests for the same. For controlled gates, the inverse is auto calculated based on the target gate. Closes #301 - Renamed `unitary_gate` to `get_unitary_gate` and added checks for all the input arguments. - The control value for a gate is now baked into the control gate itself. The user can always generate his own version of controlled gate e.g. `controlled(gates.X, num_ctrl_qubits=2, control_value=0b10)`. For more detail check out #308 (comment) - For all Controlled gates, the (target_gate, num_ctrl_qubit, ctrl_value) is used as another key in the namespace to reference that controlled gate. This is done in order to prevent regeneration of the controlled gate class for a given target gate with known (num_ctrl_qubit, ctrl_value). Since the namespace is used by default, there is no overhead. - Renamed `controlled_gate` to `controlled_gate_unitary` and resolved the bug which didn't allow for generating the correct qobj if number of controls > 1. - Added gate equivalence checking for `ParametricGate`, `ControlledGate`. And added `__eq__`, `__hash__` in Gate metaclass as well. Replaced gate.name = "X" search with gate == X now. - Corrected inconsistency between Gate and Type[Gate] from previous PRs. Gate refers to an instantiated object while type[Gate] refers to a Gate class or subclass in accordance with Python's typehinting. - Added several more checks in `__init_subclass__`, `add_gate` method. - Added tests in `test_gates.py` and `test_circuit.py` for mostly incorrect cases like num_qubits being negative etc., which should raise an Error. Since Gates are used throughout the codebase, their normal usage is already mostly covered. **Maintainence and Bug Fixes:** - `gate_product` had a clear bug that `tensor_list` was not defined but was being used, fixed that and also moved the entire `gate_product` logic to operations, which was split in circuit-simulator utils, operations but was always being imported from operations as per `__init__` even in `mat_mul_simulator.py`. - Removed deprecated arguments and functions that were deprecated 3 years back. This was checked in the git commit history. - Remove mutable default values from function arguments across the codebase, this is a common Python bug. Something like targets = [], can often lead to an error. For details check out this blog https://medium.com/@matbrizolla/beware-of-the-hidden-trap-understanding-mutable-default-arguments-in-python-d10e8b0a7b72 - Corrected several `isInstance` checks in the codebase. `Iterable` and `Sequence` were being used interchangeably at several places especially in qasm. This led to several unexpected behaviours and errors during gate refactoring. - Moved pytest.ini to the root, this is what pytest themselves recommend https://docs.pytest.org/en/stable/explanation/goodpractices.html#choosing-a-test-layout-import-rules . The reason was that placing pytest.ini in `tests/` led to certain sys path issues, for reference check https://docs.pytest.org/en/stable/explanation/pythonpath.html. This caused an error when standard gates were designated to a namespace and pytest was trying to redefine them and reassign them to the same namespace. *Edit: Some more changes* - Separated IDLE (meant for Pulse level simulation) and Identity gate (standard Identity matrix). - In Pulse compiler instead of `gate.name`, `gates` are themselves being used as keys. - Removed expand argument from `inverse()` in Parametric Gates. - Renamed `controlled` to `get_controlled_gate`.
get_compact_qobjwithget_qobjand deprecated the former.is_clifford,self_inverseclass attributes toGateclass.num_ctrlsclass attribute, from Gate class toControlledGateGate class.ParametrizedGateclass to ParametricGate`.num_paramsclass attributes,validate_parametersmethod toParametricGate class.ControlledParamGateclass and included that functionality inControlledGateclass itself.Gateto disallow modification of Gate class attribution (since they are shared among all object instances).__init_subclass__inGate,ParametricGate,ControlledGateclass and several checks on class attribute type and value.unitary_gate,controlled_gate.operations/std/dir, later renamed to/operations/gatesdir in Refactor Gate class (2) #328.add_gatemethod.gates.py