Skip to content

Conversation

markjeveritt
Copy link
Contributor

Needs updating following mine and Russell's conversation with Lev to use the jobs interface and to be more general in its construction so that it can become an effective library routine rather than simple calculator.

Needs updating following mine and Russell's conversation with Lev to use the jobs interface and to be more general in its construction so that it can become an effective library routine rather than simple calculator.
@levbishop levbishop self-assigned this Mar 14, 2017
@levbishop
Copy link
Member

This is exactly the kind of thing we'd like to bring into the sdk.

@markjeveritt
Copy link
Contributor Author

Very happy to hear this - how do we proceed please?

@jaygambetta
Copy link
Member

Hi @markjeveritt I have delayed looking at this as we were changing QISKit. Now we have finished r0.2 I would like to help you add this to QISKit.

I see that you have a function for Wigner function that generates the set of experiments needed to make the plot. I would like to have this also work with data from local simulators so i want to abstract this away from the backend and make it work on data that is generated either by simulation or experiment. I think the way to do this is add this to a new folder that we call plotters or tomography as it is a more advance plotter than basic plotters (and general state tomography for example we will put in there as well) and it will have both the tools to generate the quantum program for this state and then plot this quantum program. Let me think about this for a bit and I will comment here with a suggestion soon.

@awcross1 and @ismaelfaro what do you think. Do you feel that all tomography tools should go in a folder for tomography or are the a method for a quantum program to plot or a tool extension like the optimization one i am currently working on.

I have a concern that since we changed QISKit so much it might be hard to merge this in or make edits on it so I might have to copy this into a working branch to work with the new version of QISKit and close this request to get it in.

Creates GHZ states but may not work for two qubits and has problems above five.
@markjeveritt
Copy link
Contributor Author

markjeveritt commented May 25, 2017

Hi @jaygambetta - we guessed something like this had caused the delay - no problem - we were very excited to hear the 16/17 qubit announcement. Russell Rundle has been looking at this and we think we have batched code that will work with r0.2. It works for five quits but we have been having trouble getting it to work in the simulator for more (we were hoping to rush out a nice example demonstrating your machine could make a 16qubit GHZ or something like this). We have updated the code to reflect these changes (example plot attached)

I agree we want to abstract away from the backend - its good practice to separate concerns.

A folder for plotters and tomography would make sense but just so you know we think we may be able to also derive some good new entanglement measures (working on this with Todd Tilma but its not ready yet). Not sure if this effects your planning (folder by method, application or more layers?).

three_qubits_wigner_function_equatorial_plot

@chriseclectic
Copy link
Member

chriseclectic commented Aug 10, 2017

Hi @markjeveritt,

Now that we have the next release up we can come back to this, sorry for the wait! Unfortunately some things have changed in how we’ve structured qiskit so this would require some modifications. Some of the functions themselves would be good to add some to our new tools modules with a few changes, and then if you’re interested it would be nice for you to make a Jupyter notebook demonstrating this which could be added to qiskit-tutorial.

Now onto the specifics. Our philosophy is for these types of modules to be abstracted in the following way:

  1. Have a function that adds the necessary quantum circuits to a quantum program (which you would execute to get the data you need)
  2. This quantum circuit can then be executed on an arbitrary backend (QX2 or simulator) to obtain a Results object (new to r0.3).
  3. Have a function which takes the Results object as input and post-processes this to obtain the desired information.
  4. Have a visualisation functions act either on arbitrary Results, or density matrices/state vectors/some other data format.

I’d suggest looking at the new tomography module and the state_tomography tutorial notebook for an idea of how to implement this. In the state tomography case we break it down into three functions:

  • build_state_tomography_circuits to generate the circuits.
  • state_tomography_data to extract tomography data from results.
  • fit_tomography_data to reconstruct the state.
    The state can then be visualised in various ways using the plot_state function from the visualization module.

For the Wigner code you code I would suggest breaking this into something like
build_wigner_circuits, wigner_data, then for plotting adding a plot_wigner_function to the visualization module. I’d also recommend adding a wigner_function to tools.qi module for computing the winger function for an arbitrary quantum state.

For specific comments for you code:

  • Please format as a module so that it can be imported (as apposed to just run as command line scripts). For script style ones they would make more sense to be converted to a tutorial Jupyter notebook for the qiskit-tutorials.
  • You shouldn’t need to access the API directly, this is all handled by using the QuantumProgram class from this sdk. I’d then recommend building the circuits using circuit objects (again you can see an example of this in the tomography module).
  • Ideally there should be some basic unit tests for the main functions too so that we can verify they pass.

The mundane: to keep the codebase organised and well documented we need

  • Please send pull-requests to QISKit:dev not the master.
  • PEP8 python format (you can use pylint, flake8, or similar plugin to check this in your editor)
  • Include Google style docstrings for each function/class/module .
  • We’ve decided to remove authors/affiliation information from individual files.

After all that, we look forward to your resubmission to help flesh out the qiskit tool! (Sorry if it was a bit long winded) If you need help or have further questions with regards to any of this feel free to send me an email or comment here.

Cheers

@markjeveritt
Copy link
Contributor Author

markjeveritt commented Aug 16, 2017

Hi @chriseclectic

No worries - I quite understand. Just to say we are working on it. Thank you for the detailed instructions of what we need to do.

We are not masters of Git - how do we make sure that our pull request goes to QISKit:dev please?

Also - would it be ok to include author information in the Jupiter notebook or somewhere else please (some record of our contribution would be potentially useful for us for a research assessment exercise we have to go though here).

Cheers

Mark

@chriseclectic
Copy link
Member

Hi @markjeveritt,

You can edit the pull request to change branch like this. Though it might make more sense to close this request and start a new one when you're ready.

I believe its fine to include author/affiliation information in an example Jupyter notebook in the qiskit-tutorials, and we will maintain a list of contributors to this repo with the Github history keeping track of who submits what.

Cheers,
Chris

@markjeveritt
Copy link
Contributor Author

Hi @chriseclectic

I am trying to upload our files but cannot see the branch QISKit:dev (I see other branches but not this one). What should we do please?

Best wishes

Mark

@diego-plan9
Copy link
Member

Hi @markjeveritt ,

there have been a number of changes related to the organization of the repository - in practice, it means your pull request should be issued against master (ie. the same way it is now!), as master is now considered the development branch.

However, there have also been a number of commits and changes since your PR was issued, which caused your fork's master branch to be out of sync with the main master branch - it would be a good idea to try to get them back in sync before proceeding with the rest of the changes, in particular as there has been changes on the tools folder and qiskit.tools modules. Would it be possible to:

  1. Sync your fork with the current version of the SDK - we can actually do it ourselves on your fork if that is more convenient to you (you would still need to update your local-machine repository, though)
  2. Proceed with the changes mentioned by Chris at Adds a simple spin Wigner function calculator example #3 (comment), which should still be relevant - the main difference is that your code should be included in /qiskit/tools, not /tools.
  3. Update the pull request with the new changes (just by pushing to your fork).

Alternatively, as Chris and Jay suggested, it might be cleaner to just "start from scratch" on a new fork, and issue a new PR - please let us know if you need any comments at any step!

@markjeveritt
Copy link
Contributor Author

Opening new pull request to try again

@diego-plan9
Copy link
Member

Re-issued as #132.

ajavadia referenced this pull request in ajavadia/qiskit Mar 26, 2018
taalexander referenced this pull request in taalexander/qiskit-terra Apr 13, 2019
rename channel bank->store and change its interface
taalexander referenced this pull request in taalexander/qiskit-terra May 2, 2019
Cryoris referenced this pull request in Cryoris/qiskit-terra Mar 29, 2020
mergify bot added a commit that referenced this pull request Dec 4, 2020
* optimize qubit usage in Pwl

* optimize helper qubits in poly PR

* optimize weighted adder

* add reno

* Optimize ancilla (#3)

* added a method that implements piecewise polynomials that extends the piecewise linear class functionality, and a class to approximate the inverse function

* release notes

* updated header

* removed header encoding

* fixed ancilla

* remove old files

* Apply suggestions from code review

Co-authored-by: Julien Gacon <gaconju@gmail.com>

Co-authored-by: Julien Gacon <gaconju@gmail.com>

* don't include last breakpoint

Co-authored-by: Luciano Bello <luciano.bello@ibm.com>
Co-authored-by: Almudena Carrera Vazquez <almudenacarreravazquez@hotmail.com>
Co-authored-by: Kevin Krsulich <kevin.krsulich@ibm.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
mtreinish referenced this pull request in mtreinish/qiskit-core Mar 9, 2021
alexanderivrii referenced this pull request in alexanderivrii/qiskit-terra Oct 7, 2021
first pass on adding CircuitElement mixin to Clifford by @alexanderivrii
Zoufalc pushed a commit to Zoufalc/qiskit-terra that referenced this pull request Dec 13, 2021
SamFerracin pushed a commit to SamFerracin/qiskit that referenced this pull request May 20, 2025
…tes-new

adding commutes method to QubitSparsePauli
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants