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

Integrate Fermionic QAOA(FQAOA) Module and Associated Files into OpenQAOA #322

Open
wants to merge 26 commits into
base: dev
Choose a base branch
from

Conversation

yoshioka1128
Copy link

@yoshioka1128 yoshioka1128 commented Aug 23, 2024

Description

This pull request introduces the FQAOA (Fermionic Quantum Approximate Optimization Algorithm) module along with its associated files into the OpenQAOA framework. The integration includes the following components:

  • fqaoa_workflow.py: Implements the FQAOA workflow, enabling the use of fermionic methods within the OpenQAOA framework.
  • fqaoa_utils.py: Provides utility functions to support the FQAOA implementation, including essential operations and helper functions.
  • test_fqaoa.py: Contains unit tests for the fqaoa_utils.py, ensuring the correctness and reliability of the new functionality.
    test_workflows.py: Tests the overall workflow integration, confirming that FQAOA integrates smoothly with existing components.
  • test_analytical_simulator.py: Includes tests for the analytical simulator, verifying the exception handling in the FQAOA approach.
  • 16_FQAOA_examples.ipynb: A Jupyter notebook demonstrating an example of solving the portfolio optimization problem using FQAOA. This notebook showcases the practical application of the FQAOA method and provides a step-by-step guide for users.

All tests have been successfully executed, demonstrating the stability and effectiveness of the integrated features. This addition enhances the OpenQAOA framework by introducing the capability to handle constrained combinatorial optimization problems using FQAOA.

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)

How Has This Been Tested?

The following unit tests have been added to ensure the robustness and accuracy of the new features:
test_fqaoa.py: Tests the core functionality of the fqaoa_utils.py.

test_workflows.py: Validates the integration of fqaoa_workflow.py within the overall workflow.

test_analytical_simulator.py: Confirms the exception handling in the FQAOA approach.

Sorry, something went wrong.

@yoshioka1128 yoshioka1128 force-pushed the yoshioka1128/dev_clean_fqaoa branch from 633f28e to 31da142 Compare August 26, 2024 08:40
@yoshioka1128
Copy link
Author

I've just pushed an additional notebook 17_FQAOA_advanced_parameterization.ipynb to this branch that provides a tutorial on advanced circuit parametrization, including Fourier parametrization for FQAOA. This notebook is intended to demonstrate more complex usage scenarios of FQAOA and complement the existing documentation.

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.

Good job on the implementation! While the PR is pretty big I think you still managed to get something really neat and relatively easy to review.
The documentation is really helpful, you did a great lot of work on this!

There are a few typos I commented on, and other minor changes I'd like you to implement. Feel free to discuss on the comments directly.

Also note that I don't know some of the functions like Givens rotation related algorithms, I trust that you implemented them correctly.

docs/source/index.rst Outdated Show resolved Hide resolved
docs/source/index.rst Outdated Show resolved Hide resolved
examples/16_FQAOA_examples.ipynb Outdated Show resolved Hide resolved
examples/16_FQAOA_examples.ipynb Outdated Show resolved Hide resolved
examples/16_FQAOA_examples.ipynb Outdated Show resolved Hide resolved
src/openqaoa-core/tests/test_workflows.py Outdated Show resolved Hide resolved
src/openqaoa-core/tests/test_workflows.py Outdated Show resolved Hide resolved
src/openqaoa-core/tests/test_workflows.py Outdated Show resolved Hide resolved
src/openqaoa-core/tests/test_workflows.py Outdated Show resolved Hide resolved
src/openqaoa-core/tests/test_workflows.py Outdated Show resolved Hide resolved
@yoshioka1128
Copy link
Author

Thank you for your review and the kind words!
I appreciate your feedback and will go through the comments to make the suggested changes.
I’ll also review the typos and other minor issues you pointed out.

I will pay particular attention to the functions, such as givens rotation, to make sure that they are correct.

Thanks again for your time and valuable feedback!

@yoshioka1128
Copy link
Author

Thank you for the detailed review! I've pushed a new commit that addresses the typos and minor corrections as per your comments. Here's a summary of the changes:

  • Fixed typos and made small corrections based on the review feedback.
  • Updated variable names and improved documentation for clarity and consistency.

Please note that the error mitigation SPAM twiling is still pending and will be tackled in a separate commit.

I've also performed a final spell check using codespell and aspell to ensure there are no lingering issues. I apologize for any inconvenience caused by the previous commits. Let me know if you have any further suggestions or if anything needs additional adjustments!

@yoshioka1128 yoshioka1128 force-pushed the yoshioka1128/dev_clean_fqaoa branch from 386824e to 3769a86 Compare September 6, 2024 23:03
@yoshioka1128 yoshioka1128 force-pushed the yoshioka1128/dev_clean_fqaoa branch from 3769a86 to 9c607cc Compare September 8, 2024 03:41
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.

Changes look good to me! I would only like to sort out the issue with the docs and a couple of tiny comments I left.

src/openqaoa-core/openqaoa/algorithms/fqaoa/fqaoa_utils.py Outdated Show resolved Hide resolved
src/openqaoa-core/tests/test_fqaoa.py Outdated Show resolved Hide resolved
src/openqaoa-core/tests/test_workflows.py Outdated Show resolved Hide resolved
src/openqaoa-core/tests/test_workflows.py Outdated Show resolved Hide resolved
src/openqaoa-core/tests/test_workflows.py Outdated Show resolved Hide resolved
…ected incorrect usage of np.ndarray for np.array in the code.
…epare is_close_statevector for potential move to utilities.py
@yoshioka1128 yoshioka1128 force-pushed the yoshioka1128/dev_clean_fqaoa branch from 84df030 to db9001b Compare September 25, 2024 01:12
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.

Two final comments, it seems good otherwise!

src/openqaoa-core/openqaoa/utilities.py Outdated Show resolved Hide resolved
src/openqaoa-core/openqaoa/utilities.py Show resolved Hide resolved
@yoshioka1128
Copy link
Author

I want to express my sincere gratitude for your incredibly detailed review.
Your comments have been invaluable in helping me improve the quality of this pull request.
I'm truly grateful for your time and expertise.
If you have any further concerns or suggestions, please let me know.

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.

Thank you for taking the time to address all the comments I made there. Happy to help anytime!

The PR looks good to me! Just need to remove this temporary file to make sure the docs pipeline passes.

docs/source/notebooks Outdated Show resolved Hide resolved
@yoshioka1128
Copy link
Author

I've added a risk factor to the objective function and updated the related tests.
This risk factor was included in the argument but not reflected in the objective function.

Integrate risk factor into objective function and update related tests

  • Modified the objective function to include risk factor in calculations.
  • Updated tests to account for changes in the objective function.
  • Adjusted Jupyter notebooks to reflect the new implementation of the risk factor.

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.

None yet

2 participants