refactor: Simplify some internals of qiskit_to_tk conversion
#382
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
CalMacCQ
commented
Aug 24, 2024
CalMacCQ
changed the title
qiskit_to_tk and tk_to_qiskit
|
In fact I think its better to do the type annotations updates for the whole repo in a separate PR. Clutters the diff quite a bit. See #383 |
CalMacCQ
changed the title
qiskit_to_tk and tk_to_qiskitqiskit_to_tk conversion
CalMacCQ
changed the title
qiskit_to_tk conversionqiskit_to_tk conversion
CalMacCQ
commented
Sep 9, 2024
| tkc.add_circuit(circuit, qubits) | ||
| else: | ||
| raise TypeError( | ||
| "Unrecognised type of Instruction.params " |
There was a problem hiding this comment.
I'm not so satisfied with this _add_state_preparataion function. There are at least 3 different ways to create a StatePreparation or Initialize object. Couldn't simplify the handling all that much.
cqc-alec
reviewed
Sep 10, 2024
Comment on lines
298
to
299
| known_optype = _known_qiskit_gate[c_gate.base_class] | ||
| return known_optype |
There was a problem hiding this comment.
Suggested change
| known_optype = _known_qiskit_gate[c_gate.base_class] | |
| return known_optype | |
| return _known_qiskit_gate[c_gate.base_class] |
Comment on lines
362
to
369
| else: | ||
| if isinstance(prep.params[0], complex): | ||
| # convert int to a binary string and apply X for |1> | ||
| integer_parameter = int(prep.params[0].real) | ||
| bit_string = bin(integer_parameter)[2:] | ||
| circuit = _string_to_circuit(bit_string, prep.num_qubits, qiskit_prep=prep) | ||
| tkc.add_circuit(circuit, qubits) | ||
| else: |
There was a problem hiding this comment.
Use elif and else here to save indentation?
There was a problem hiding this comment.
Thanks, done in 37de5e4
cqc-alec
approved these changes
Sep 10, 2024
5 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR begins a refactor of the
qiskit_to_tkandtk_to_qiskitconverters. Here I've focussed on refactoring some of the largeCircuitBuilder.add_qiskit_datamethod (used as part ofqiskit_to_tkinto separate internal functions. I've added some short docstrings so its hopefully clearer whats going on.There's still a lot to improve in the conversion code but I think it makes sense to do this refactor in several stages to make small fixes like #366 a bit easier.
I've avoided imporvements which involve user facing changes for now. Will add these in a follow up.
Related issues
#313
Checklist