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

Use repeat in QuantumCircuit.power if power is numpy.integer #6910

Merged
merged 1 commit into from
Aug 16, 2021

Conversation

t-imamichi
Copy link
Member

Summary

Follow-up of #6831
Fixes qiskit-community/qiskit-optimization#225. Qiskit Optimization unit test of GroverOptimizer takes so long time after the merge of #6831.

This PR resolves it by applying repeat if power is np.integer.

Details and comments

GroverOptimizer of Qiskit optimization set power by numpy.random.Generator.integers. The type is numpy.int64. So, it is necessary to use np.integer for isintance to catch this case.
https://github.com/Qiskit/qiskit-optimization/blob/4953e56e3c062df42e7e51fa6b89172e1f69e3c1/qiskit_optimization/algorithms/grover_optimizer.py#L220-L232

https://numpy.org/doc/stable/reference/random/generated/numpy.random.Generator.integers.html

@1ucian0 1ucian0 added the Changelog: None Do not include in changelog label Aug 16, 2021
@1ucian0 1ucian0 merged commit 1bdc6f0 into Qiskit:main Aug 16, 2021
@t-imamichi t-imamichi deleted the fix-power branch August 16, 2021 09:10
@jakelishman jakelishman added the stable backport potential The bug might be minimal and/or import enough to be port to stable label Aug 16, 2021
mergify bot pushed a commit that referenced this pull request Aug 16, 2021
(cherry picked from commit 1bdc6f0)

# Conflicts:
#	qiskit/circuit/quantumcircuit.py
@jakelishman
Copy link
Member

This was actually a mistake in #6831 - the line previously had numbers.Integral (which is what this PR should have changed it to as well), and I mistakenly changed it to int for some reason (possibly a mistake because int in type hints is equivalent to isinstance(x, numbers.Integral)). We should have the general form back, really.

@jakelishman jakelishman removed the stable backport potential The bug might be minimal and/or import enough to be port to stable label Aug 16, 2021
@t-imamichi
Copy link
Member Author

Thank you for your information. It's OK to change the code to isinstance(x, numbers.Integral).

@jakelishman
Copy link
Member

I'm making another PR that's (hopefully) going to convert most of our isinstance(x, int) checks into isinstance(x, numbers.Integral), where that makes sense.

@kdk kdk added this to the 0.19 milestone Nov 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: None Do not include in changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Scheduled CI failed
4 participants