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 random statevector distribution #10866

Merged
merged 5 commits into from
Sep 22, 2023
Merged

Conversation

kevinsung
Copy link
Contributor

Summary

Fixes #9619

Details and comments

@kevinsung kevinsung requested review from ikkoham and a team as code owners September 20, 2023 12:41
@qiskit-bot
Copy link
Collaborator

One or more of the the following people are requested to review this:

  • @Qiskit/terra-core
  • @ikkoham

@mtreinish mtreinish added Changelog: Bugfix Include in the "Fixed" section of the changelog mod: quantum info Related to the Quantum Info module (States & Operators) labels Sep 20, 2023
@mtreinish mtreinish added this to the 0.25.2 milestone Sep 20, 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 this. I think the output from your fixed version is Haar random, isn't it? - in the sense of the result is distributed the same as if we applied a Haar-random unitary to some fixed statevector. I'm not certain of that, but if so, it might be good to keep some text describing that in the documentation. If not, then I think we still might need to define what we mean "uniform distribution".

Could we add a bugfix note about fixing the distribution? I'm feeling about iffy about backporting this, even if we call it a bugfix, just because of the (intentional!) RNG incompatibility - I think personally I'd prefer to wait til a minor with it.

@jakelishman jakelishman modified the milestones: 0.25.2, 0.45.0 Sep 20, 2023
@kevinsung
Copy link
Contributor Author

I don't think we need to mention the Haar measure on unitary matrices. This is the uniform distribution. I've added a citation for it.

jakelishman
jakelishman previously approved these changes Sep 21, 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.

Ah super thanks - this reads well to me. Julien, can you tag for merge if you're happy as well?

@Cryoris Cryoris added this pull request to the merge queue Sep 22, 2023
Merged via the queue into Qiskit:main with commit 364debc Sep 22, 2023
14 checks passed
@kevinsung kevinsung deleted the random-statevector branch September 22, 2023 13:21
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 mod: quantum info Related to the Quantum Info module (States & Operators)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Wrong probability distribution for random_statevector()
5 participants