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

optimize FidelityQuantumKernel parameterization #719

Conversation

MoritzWillmann
Copy link
Contributor

Summary

This PR optimizes the way indices, right_parameters and left_parameters are created in _get_symmetric_parameterization and _get_parameterization of the FidelityQuantumKernel class.

Details and comments

The previous version used a heavy amount of dynamic memory allocation which resulted in a very long runtime. This new version runs way faster for bigger instances by not using dynamic memory allocation. I didn't add any additional tests, since this doesn't introduce any new functionality.

I'm happy to incorporate comments and hope this can be merged soon 😄

@adekusar-drl
Copy link
Collaborator

Can you please measure how faster the implementation becomes with this PR?

@coveralls
Copy link

coveralls commented Nov 27, 2023

Pull Request Test Coverage Report for Build 7049710944

  • 10 of 10 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.03%) to 92.66%

Totals Coverage Status
Change from base Build 7049706485: -0.03%
Covered Lines: 1843
Relevant Lines: 1989

💛 - Coveralls

@MoritzWillmann
Copy link
Contributor Author

Can you please measure how faster the implementation becomes with this PR?

This is the runtime of the respective functions in seconds as benchmarked with timeit with x_vec.shape==y_vec.shape==(100,2) and evaluate_duplicates=="all" and 100 executions:

get_parameterization            Old     11.828130300000339
get_parameterization            New     0.4398030000011204
get_symmetric_parameterization  Old     4.378354100001161
get_symmetric_parameterization  New     0.23957780000091589

@adekusar-drl
Copy link
Collaborator

Thanks, this is interesting. Can you please additionally measure an overall impact on qk.evaluate(), especially if there are more than 5 features/qubits?

@MoritzWillmann
Copy link
Contributor Author

With x_vec.shape==y_vec.shape==(100,7), ZFeatureMap(7, reps=2), ComputeUncompute(Sampler()), evaluate_duplicates="all" and timeit with 10 executions, in seconds:

Old asymmetric  734.5568785999985
New asymmetric  702.9984134999995
Old symmetric   348.1263235000006
New symmetric   346.7739440999994

Copy link
Collaborator

@adekusar-drl adekusar-drl left a comment

Choose a reason for hiding this comment

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

So, performance gain is not that large on a decent number of qubits, but still there is.

Copy link
Collaborator

@adekusar-drl adekusar-drl left a comment

Choose a reason for hiding this comment

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

Thanks, looks good

@adekusar-drl adekusar-drl merged commit ba89cc4 into qiskit-community:main Dec 1, 2023
15 checks passed
@MoritzWillmann MoritzWillmann deleted the optimize_fqk_parameterization branch December 8, 2023 08:25
oscar-wallis pushed a commit that referenced this pull request Feb 16, 2024
* optimize fqk parameterization

* Update qiskit_machine_learning/kernels/fidelity_quantum_kernel.py

Co-authored-by: Anton Dekusar <[email protected]>

* Update qiskit_machine_learning/kernels/fidelity_quantum_kernel.py

* remove vstack

---------

Co-authored-by: Anton Dekusar <[email protected]>
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.

3 participants