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

Integration of Mitiq's ZNE #302

Merged
merged 71 commits into from
May 28, 2024
Merged

Conversation

victor-onofre
Copy link
Contributor

Description

  • Integrate Mitiq into OpenQAOA to allow error mitigation techniques to take place during the execution of the QAOA. We have created a wrapper focusing on the ZNE technique following a similar structure to the SPAM Twirling wrapper.
  • This is a project born from the Quantum Open Source Foundation Mentorship program
  • New dependency to add: Mitiq
  • Fixes Integrate ZNE from Mitiq into OpenQAOA #207

Checklist

  • I have performed a self-review of my code.
  • I have commented my code and used numpy-style docstrings
  • I have made corresponding updates to the documentation.
  • My changes generate no new warnings
  • I have added/updated tests to make sure bugfix/feature works.
  • New and existing unit tests pass locally with my changes.

Type of change

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)
  • This change requires a documentation update

How Has This Been Tested?

We have been testing the wrapper with the Max Cut problem but we need help on defining unit-tests. What will be the best test to add?

KilianPoirier and others added 30 commits December 20, 2023 16:38
calibration data structure for zne.
* 'available_error_mitigation_techniques' list now includes 'mitiq_zne'
* compile() from QAOA class now creates a ZNEWrapper
* Some small fixes over ZNEWrapper class.
@victor-onofre
Copy link
Contributor Author

Hi @KilianPoirier,

All the comments have been addressed, and the dev branch has been merged. We have added two tests similar to the SPAM wrapper, it is still necessary to add a couple more, and we plan to do it next week. I think you can now review all these changes.

The example notebook is a good idea. Do we do that in this PR or create another one?

Thanks!

Copy link
Collaborator

@KilianPoirier KilianPoirier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few more comments here.
I approved the test pipeline run, so you'll have more info if the test are passing or not now that you have merged dev back into your branches.

Edit: Please add mitiq to the requirement file.

@KilianPoirier
Copy link
Collaborator

Concerning the example notebook, feel free to add them to this PR since everything is related to the feature you're developing.

@victor-onofre
Copy link
Contributor Author

Hi @KilianPoirier,

I have added mitiq to the requirements, but the tests are still waiting for approval. Some of the previous errors were related to that issue.

We are going to work on the example notebooks and the rest of the tests.

Thanks!

@KilianPoirier
Copy link
Collaborator

Hi @victor-onofre ,

Thanks for updating the requirements.
Good news, the tests are passing! Let me know when you implement your own tests and the example notebook.

@victor-onofre
Copy link
Contributor Author

Hi @KilianPoirier! We have push an update with the example notebook and the tests. Now is ready for another review.

Thanks!

@KilianPoirier
Copy link
Collaborator

Hey @victor-onofre , it seems the regular tests are failing now.

It is most likely due to the use of images in your example notebook. Can you try copying the images directly in the notebook without using PIL?
I believe that if the notebook is ran correctly and the image is resolved, the image itself will be tracked in the notebook without a need to import it.

I will also have a look at the docs and let you know if I find anything.

Copy link
Collaborator

@KilianPoirier KilianPoirier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only minor comments. Good job on the implementation of tests and the recent changes!

I have a clue where the issue lies for the docs tests, try to add the following line in the toctree openqaoa/docs/source/index.rst, l.306

   notebooks/15_Zero_Noise_Extrapolation.ipynb

src/openqaoa-core/openqaoa/backends/wrapper.py Outdated Show resolved Hide resolved
src/openqaoa-qiskit/tests/test_qiskit_backend_wrapper.py Outdated Show resolved Hide resolved
@AdrianoLusso
Copy link
Contributor

Hi @KilianPoirier! We have followed the two recommendations you give us to solve the docs issue (not using PIL and adding the line in openqaoa/docs/source/index.rst). We have also solved the other secondary issues.

Thanks for your feedback!

Copy link
Collaborator

@KilianPoirier KilianPoirier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

@KilianPoirier KilianPoirier merged commit 7f84b11 into entropicalabs:dev May 28, 2024
4 checks passed
@KilianPoirier KilianPoirier changed the title Dev Integration of Mitiq's ZNE May 28, 2024
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.

Integrate ZNE from Mitiq into OpenQAOA
4 participants