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

plot_bloch_vector (and others) no longer directly importable from qiskit.tools.visualization #847

Closed
dtmcclure opened this issue Sep 1, 2018 · 6 comments · Fixed by #863
Labels
bug Something isn't working

Comments

@dtmcclure
Copy link
Member

dtmcclure commented Sep 1, 2018

Informations

  • Qiskit Terra version: 0.5.7
  • Python version: 3.6.5
  • Operating system: Windows 7

What is the current behavior?

The command from qiskit.tools.visualization import plot_bloch_vector no longer works now that this function is defined in the file _state_visualization.py in the visualization folder, rather than in the file visualization.py. Perhaps this is intentional, but in that case then the file qiskit/python/quantum_phase_bloch.py in the repository ibmqx-user-guides should be updated accordingly.

Steps to reproduce the problem

  1. Start python
  2. Run from qiskit.tools.visualization import plot_bloch_vector
  3. Observe error message

What is the expected behavior?

  1. I expect it to import the function and not give an error message.

Suggested solutions

  1. Add ", plot_bloch_vector" (and possibly other functions) to the end of line 12 in qiskit.tools.visualization.__init__.py.
  2. Alternatively, change line 5 of qiskit/python/quantum_phase_bloch.py in the ibmqx-user-guides repository to read "from qiskit.tools.visualization._state_visualization import plot_bloch_vector"
@delapuente delapuente added the bug Something isn't working label Sep 3, 2018
@diego-plan9
Copy link
Member

diego-plan9 commented Sep 4, 2018

Nice catch @dtmcclure ! I'm actually unsure about the best solution: perhaps the best way is to update the ibmqx-user-guides file, but instead of modifying the import for pointing to the specific function (as technically modules with an underscore are "private"), make the file use plot_state(rho, method='bloch') instead?

The reason would be that plot_state acts as a "dispatcher" function of sorts - any other state visualization can be invoked through it ... and it seems nor the tutorials or the user guides are using any other function defined in _state_visualization. I'm unsure if this was by convention, though: probably a question for the tutorials/user guides?

@jaygambetta
Copy link
Member

We want the plot_bloch_vector as a function.

@ajavadia
Copy link
Member

ajavadia commented Sep 5, 2018

This was done to give users one unified interface for plotting density matrices:

plot_state(rho, method)

where method can be "bloch", "city", "qsphere" or "paulivec". The idea is that these all plot the same thing, in different ways.

So I would also vote for keeping that the "public" interface, and the underlying methods as private.

@jaygambetta
Copy link
Member

jaygambetta commented Sep 5, 2018

yeah, I agree but the point is you can plot a Bloch sphere with much less information than a state as it is not informationally complete. I am fine with the others not being accessed unless using state but this one I think should be special. I can't believe i'm defending the Bloch sphere when I think this is the worst thing physicists ever got used to helping explain quantum computing.

The plot_state method should only use rho but the plot_vector uses the smaller vector format. The reason is also when using it for education i dont want to make a rho when we dont need to.

@ajavadia
Copy link
Member

ajavadia commented Sep 5, 2018

Thanks for the clarification. I made a PR to make the plot_bloch_vector method public, but the rest remain accessible through plot_state.

@jaygambetta
Copy link
Member

@ajavadia we can close this correct?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants