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

Fix tolerance when checking if diagonal is unitary #707

Merged
merged 1 commit into from
Apr 20, 2020

Conversation

chriseclectic
Copy link
Member

Summary

Fixes #705

Fixes bug where diagonal gates are flagged as non-unitary due to being checked with too high a tolerance.

Adds extra diagonal gate tests

Details and comments

@chriseclectic chriseclectic added the stable-backport-potential The issue or PR might be minimal and/or import enough to backport to stable label Apr 20, 2020
@chriseclectic chriseclectic added this to the Aer 0.5.1 milestone Apr 20, 2020
@chriseclectic chriseclectic requested a review from atilag as a code owner April 20, 2020 15:45
@chriseclectic chriseclectic force-pushed the fix/diagonal-is-unitary branch from 14563ca to 6fbac6d Compare April 20, 2020 15:55
@chriseclectic chriseclectic merged commit 7de864c into Qiskit:master Apr 20, 2020
chriseclectic added a commit to chriseclectic/qiskit-aer that referenced this pull request Apr 20, 2020
@adcorcol
Copy link
Member

adcorcol commented Apr 21, 2020

Maybe I'm doing something wrong, but I have implemented your change
for (const auto val : op.params) { if (!Linalg::almost_equal(std::abs(val), 1.0, 1e-7)) { throw std::invalid_argument("\"diagonal\" matrix is not unitary."); } }

within json_to_op_diagonal in operations.hpp and I still keep getting a "diagonal" matrix is not unitary' error. It does work for diagonals up to 2 or 3 qubits but fails for 4 qubits and beyond. I have tried toggling the tolerance a bit but still no success. Could this error be coming from somewhere else? Or maybe I need to change some other file that I did not notice?

(For example this 4-qubit diagonal

array([1. +0.j , 0.99999171-0.00407149j, 0.99999534-0.00305362j, 0.99991245-0.013232j , 0.99998337-0.00576793j, 0.99995159-0.00983931j, 0.99979279-0.02035611j, 0.9995338 -0.03053154j, 0.99981518-0.01922536j, 0.99950577-0.03143588j, 0.99975181-0.02227833j, 0.99917585-0.04059079j, 0.99798788-0.06340504j, 0.99713899-0.07558995j, 0.99695665-0.07795798j, 0.99536107-0.09620988j])

fails)

@chriseclectic
Copy link
Member Author

@adcorcol For your 4-qubit example I am getting in error from the terra diagonal gate class that one of the entries does not have absolute value 1, so I think it's failing before it gets to the simulator.

Looking at the diagonal gate class it is checking with tolerance 1e-10, which is too high for Numpy. I think its worth raising an issue for that on Qiskit Terra.

@adcorcol
Copy link
Member

Thanks, I will look into it and submit an issue.

@chriseclectic chriseclectic deleted the fix/diagonal-is-unitary branch April 28, 2020 19:19
@chriseclectic chriseclectic added the Changelog: Bugfix Include in the Fixed section of the changelog label Aug 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: Bugfix Include in the Fixed section of the changelog stable-backport-potential The issue or PR might be minimal and/or import enough to backport to stable
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Diagonal operator flagged as not unitary
2 participants