Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding global_phase gate in qiskit-terra #9251

Merged
merged 93 commits into from
Apr 17, 2023

Conversation

sumit-kale
Copy link
Contributor

Summary

Added global phase gate (global_phase) which can be applied to a:class:~qiskit.circuit.QuantumCircuit
This PR fixes Global phase gate #8236 issue.

Details and comments

  • Added relevant test functions in test_extensions_standard.py
  • Release notes have been added
  • Passes lint checks locally

sumit-kale and others added 30 commits October 10, 2022 22:53
@sumit-kale
Copy link
Contributor Author

It looks like Jake's fix #9899 resolves the issue. It would be good to add a test case for that.

I have added the test case for that! Thanks for the suggestion.

Copy link
Member

@jakelishman jakelishman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like you've had some problems with merge conflicts in the places I've highlighted here - are you able to fix them yourself, or do you need a hand?

qiskit/circuit/library/standard_gates/__init__.py Outdated Show resolved Hide resolved
Comment on lines 78 to 83
<<<<<<< HEAD
=======
Standard gates
>>>>>>> 5672c70649b4f83a6c7146c68a6a543d4a3908fd
=======
>>>>>>> 57939ba279d95b5e3f56a08e186f5e1459c3c44f
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

Copy link
Member

@jakelishman jakelishman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for looking at this. I've just got a couple of book-keeping comments, nothing meaningful about your new gate itself. If you're able to get these changes fixed in the next week, we'll release this as part of Qiskit Terra 0.24 soon!

If you need a hand / don't have time, don't worry - just let me know and I'll fix up the last few things myself and merge it.

qiskit/circuit/library/standard_gates/__init__.py Outdated Show resolved Hide resolved
qiskit/circuit/library/standard_gates/global_phase.py Outdated Show resolved Hide resolved
qiskit/circuit/library/standard_gates/global_phase.py Outdated Show resolved Hide resolved
test/python/circuit/test_extensions_standard.py Outdated Show resolved Hide resolved
@jakelishman
Copy link
Member

I've also made #9962 to track the issue of the visualisation that we discussed above. That doesn't need solving for this PR, it's just so we don't forget (as easily).

jakelishman
jakelishman previously approved these changes Apr 17, 2023
Copy link
Member

@jakelishman jakelishman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for all the work on this! I've fixed up the merge conflict that had just arisen, and I'll tag this for merge now. There's nothing more you need to do - the PR will update itself and merge when it reaches the head of the queue.

@coveralls
Copy link

coveralls commented Apr 17, 2023

Pull Request Test Coverage Report for Build 4721851712

  • 22 of 22 (100.0%) changed or added relevant lines in 3 files are covered.
  • 179 unchanged lines in 11 files lost coverage.
  • Overall coverage increased (+0.05%) to 85.83%

Files with Coverage Reduction New Missed Lines %
crates/qasm2/src/expr.rs 1 93.76%
qiskit/assembler/run_config.py 1 94.74%
qiskit/circuit/tools/pi_check.py 1 91.23%
qiskit/pulse/library/waveform.py 3 91.67%
crates/accelerate/src/sabre_swap/layer.rs 4 97.32%
crates/qasm2/src/lex.rs 4 90.89%
qiskit/compiler/assembler.py 6 96.13%
qiskit/execute_function.py 6 87.5%
qiskit/qobj/qasm_qobj.py 35 83.27%
qiskit/qobj/pulse_qobj.py 41 78.47%
Totals Coverage Status
Change from base Build 4720871645: 0.05%
Covered Lines: 70549
Relevant Lines: 82196

💛 - Coveralls

jakelishman
jakelishman previously approved these changes Apr 17, 2023
@kevinsung
Copy link
Contributor

What is the expected behavior if this gate is controlled? Does it remain a global phase or does it become a conditional global phase/relative phase?

It should be the latter. For example, if the control is a single qubit, then the result is a PhaseGate.

Please add a test that controlling a GlobalPhaseGate behaves correctly, as described in this comment.

@jakelishman
Copy link
Member

I would say that we don't 100% need to ensure that .control produces an exact class as its output (that's an implementation detail), just that the matrix output of the output comes out mathematically correct. I've added some tests of that in 64ea9e6.

Copy link
Contributor

@kevinsung kevinsung left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, thanks!

@jakelishman jakelishman added this pull request to the merge queue Apr 17, 2023
Merged via the queue into Qiskit:main with commit da92478 Apr 17, 2023
@jakelishman jakelishman added the Changelog: New Feature Include in the "Added" section of the changelog label Apr 17, 2023
king-p3nguin pushed a commit to king-p3nguin/qiskit-terra that referenced this pull request May 22, 2023
* created gphase.py file

* completed glbalphase file & test_gate_defin

* adding GphaseGate in @DaTa in test_gate_defn

* added reno add-global-phase

* added inv and matrix test in test_exten

* test_gphase_inv

* tox lint edit

* renamed GLobalPhaseGate to Global_Phase_Gate

* Changed the name to more competable GlobalPhaseGate class

* # pylint: disable=E1101

* # pylint: disable=no-member

* # pylint: error fixed

* including phase in super

* changed float to ParameterExp

* modified test_globalphaseGate

* phase to params[0]

* renamed global_phase.py to gphase

* test gphase new in extentions_std

* debug 1

* debug 2

* debug 3 instructions set

* debug 4

* debug 5

* debug 6

* debug 6 instructions set

* debug 7

* debug 7

* added gphase in quantumcircuit.py

* added method gphase in quantumcircuit.py

* test_gphase_inv

* gphase debug1

* gphase debug2

* gphase debug3

* inverse mtd in gphase.py

* added _array in gphase.py

* gphase.py array debug1

* test_matrix gphase complex dtype

* test_matrix in @DDT

* test_gphase_matrix

* changed gphase to global_phase

* removed extra comma l871

* typo in reno fixed

* modified reno

* Delete .vs directory

Deleted .vs directory

* Added test to check compatibility of gate with qc.attri

* debugged pylint error1

* debugged pylint error2

* debugged pylint error2a

* debugged -epy error debug1

* debugged -epy error debug 1

* debugged -epy error debug 2

* restored test_gate_definations

* Revert "restored test_gate_definations"

This reverts commit 642877b.

* reverted mistakely edited the file on Github

* Delete .vs directory

Deleted .vs directory

* restored test_gate_definations

* Revert "restored test_gate_definations"

This reverts commit 642877b.

* reverted mistakely edited the file on Github

* retrived test_gate_definitions

* PR modifications new

* commit feb 13

* range in append

* [] in append

* [0] in append

* [0]->[]

* [0]

* []

* Operator(qc)

* fixed test_ex_std

* resolved changes suggested by Cryolis

* Update global_phase.py

Typo in year

* Added transpiler consistancy test

* restored the docstring

* prevented cyclic imports and other changes

* Fix spacing in docs

* Fix lint

* Add test of controlling global phase

---------

Co-authored-by: Jake Lishman <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: New Feature Include in the "Added" section of the changelog Community PR PRs from contributors that are not 'members' of the Qiskit repo
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Global phase gate
9 participants