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

Add layer fidelity notebook #1

Merged
merged 9 commits into from
Apr 30, 2024

Conversation

itoko
Copy link
Contributor

@itoko itoko commented Apr 11, 2024

Add a notebook to explain how to use LayerFidelity experiment implemented in qiskit-experiments.

I've created notebooks directory and placed the notebook under the directory.
Any suggestion on directory structure? (I don't want to change directory structure afterwards as that causes broken links.)

@itoko
Copy link
Contributor Author

itoko commented Apr 16, 2024

  • Use qiskit_ibm_runtime instead of deprecating qiskit_ibm_provider

@dcmckayibm
Copy link
Collaborator

@itoko there should be an appropriate directory structure now. Can we also pull some of the helper functions into a file in the utilities directory?

@dcmckayibm
Copy link
Collaborator

gate_cost = -np.prod([1-0.8*gate_error(qs) for qs in to_edges(path)]) should be 1.2 not 0.8 and the readout_cost should be multiplied by the gate_cost. If we push these into the utilities it will be easier to comment and modify.

@dcmckayibm
Copy link
Collaborator

Other to do

  • Add qiskit copyright to the file
  • Clear the outputs before we merge so that the file contains text only

@itoko
Copy link
Contributor Author

itoko commented Apr 22, 2024

@dcmckayibm Regarding putting functions into utilities, in general, I prefer lazy refactoring, i.e. the approach of refactoring from a situation where experiment A and experiment B have similar (or same) functions to each other in order to define a better (more stable) utility function. In the case of the qubit chain selection function, what is the "best" chain may vary depending on experiments, and it's difficult to define a good common API before having at least two use cases. Do you have another experiment that requires exactly the same qubit chain selection algorithm in mind? If so, I don't mind to try to separate the function out as a utility function within this PR.

@itoko
Copy link
Contributor Author

itoko commented Apr 22, 2024

Thank you for correcting my miscalculation (0.8 -> 1.25). I'll fix it. As for readout_cost, I think we have a subtlety here. The impact of readout_cost depends on the number of layers. Also SPAM errors do not affect LF as their effect is removed in the curve fitting. That's why I didn't consider readout_cost when selecting the "best" qubit chain (I should have removed the readout_cost variable). I know we as users may want to avoid qubits with bad readout fidelities but LF is a measure only cares about gate fidelities. What do you think? @dcmckayibm

@dcmckayibm
Copy link
Collaborator

Could point, I would still multiply it but with scale factor on the exponent

e.g. total_cost = fidelity_cost * readout_cost^(1/readout_scale)

@dcmckayibm
Copy link
Collaborator

I think for now just take that cell and put in the utilities folder as a script. My main motivation there is to minimize code in the jupyter notebook since it's hard to do any linting, linking to code, version control etc. on jupyter notebooks.

@dcmckayibm
Copy link
Collaborator

Here @itoko I added a file with some of the graph utilities you were using a few more I will use later https://github.com/qiskit-community/qiskit-device-benchmarking/blob/main/qiskit_device_benchmarking/utilities/graph_utils.py

@itoko
Copy link
Contributor Author

itoko commented Apr 26, 2024

@dcmckayibm Thank you for demonstrating how functions in utilities folder should look like. From a developer's perspective, I see your points.
But still, for this particular notebook, I have some reservations on separating helper functions. If we make a notebook depends on some helper functions defined outside of the notebook, that requires one more step for users to run the notebook. They have to get a copy of the file with utility functions in advance, e.g. by cloning the entire qiskit-device-benchmarking repo (or pip installing it). It may not be a problem for other notebooks demonstrating how to use experiments defined within qiskit-device-benchmarking repo. However, exceptionally for this notebook, which demonstrates experiments in qiskit-experiments repo, I think it would be better that users can just download and run it as a stand-alone notebook without installing qiskit-device-benchmarking. (Maybe this notebook should live somewhere in qiskit-experiments repo? I'd like to hear others' opinions).

@itoko
Copy link
Contributor Author

itoko commented Apr 26, 2024

I've updated the logic for the best qubit chain selection in 7531adc:

  • Fixed the const for converting the avg gate infidelity to the process fidelity (and set the minimum fidelity to 0.25).
  • Fixed the scaling with max_duration to be gate_fidelity ** scale not gate_error * scale.
  • 2x speed up of flatten by removing s-t symmetry in all pair paths
  • Added cutoff option to flatten function so to limit the number of paths for each s-t pairs
    (In fact, in current pinguino1 coupling graph, there exists 10597506 candidate paths and the best selection takes more than 20 mins (estimated 50 mins)

@itoko
Copy link
Contributor Author

itoko commented Apr 26, 2024

Add qiskit copyright to the file

Qiskit 1.0 no longer have qiskit.tools.jupyter module so we cannot use qiskit_copyright function to print the copyright. Is it OK to copy the HTML text in the last markdown cell manually? (Some qiskit repo seems to stop to have the copyright in notebooks: https://github.com/Qiskit/qiskit-aer/blob/main/docs/tutorials/1_aersimulator.ipynb) Any suggestion?

@dcmckayibm
Copy link
Collaborator

Oh sorry, I meant to post this from luciano

import datetime
from IPython.display import HTML, display

def qiskit_copyright(line="", cell=None):
    """IBM copyright"""
    now = datetime.datetime.now()

    html = "<div style='width: 100%; background-color:#d5d9e0;"
    html += "padding-left: 10px; padding-bottom: 10px; padding-right: 10px; padding-top: 5px'>"
    html += "<p>&copy; Copyright IBM 2017, %s.</p>" % now.year
    html += "<p>This code is licensed under the Apache License, Version 2.0. You may<br>"
    html += "obtain a copy of this license in the LICENSE.txt file in the root directory<br> "
    html += "of this source tree or at http://www.apache.org/licenses/LICENSE-2.0."

    html += "<p>Any modifications or derivative works of this code must retain this<br>"
    html += "copyright notice, and modified files need to carry a notice indicating<br>"
    html += "that they have been altered from the originals.</p>"
    html += "</div>"
    return display(HTML(html))

qiskit_copyright()

@dcmckayibm
Copy link
Collaborator

Ok, I see your motivation for having a standalone notebook. One of the purposes of this repo was to host notebooks for qiskit-experiments (something that is not done in that repo).

So, look... we can leave the functions in there. However, you can see how there's no ability then to re-use them from other notebooks, so I will (after the merge) try and mirror some of those functions in the graph_utilities file.

@itoko
Copy link
Contributor Author

itoko commented Apr 30, 2024

I think it's now ready to be merged.

@dcmckayibm
Copy link
Collaborator

Ok thanks @itoko

@dcmckayibm dcmckayibm merged commit 75b2533 into qiskit-community:main Apr 30, 2024
1 check passed
@itoko itoko deleted the layer-fidelity-notebook branch April 30, 2024 15:20
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.

2 participants