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

Improved support of parametrized gates in DDSIM Backends #293

Merged
merged 16 commits into from
Sep 28, 2023
Merged

Improved support of parametrized gates in DDSIM Backends #293

merged 16 commits into from
Sep 28, 2023

Conversation

andresbar98
Copy link
Contributor

@andresbar98 andresbar98 commented Sep 12, 2023

This PR expands the handling of parameterized circuits by DDSIM backends. DDSIM backends can now bind parameters to quantum circuits, resembling the way Qiskit Primitives operate.

  • A list containing the corresponding parameter values can be entered as an extra argument in the run() method
  • Add corresponding tests to this new feature

Solves #295 first task.

@codecov
Copy link

codecov bot commented Sep 12, 2023

Codecov Report

Merging #293 (6fa93f7) into main (af65305) will increase coverage by 0.0%.
Report is 2 commits behind head on main.
The diff coverage is 100.0%.

Additional details and impacted files

Impacted file tree graph

@@          Coverage Diff          @@
##            main    #293   +/-   ##
=====================================
  Coverage   92.5%   92.5%           
=====================================
  Files         32      32           
  Lines       2537    2553   +16     
  Branches     351     351           
=====================================
+ Hits        2348    2363   +15     
- Misses       189     190    +1     
Flag Coverage Δ *Carryforward flag
cpp 94.6% <ø> (-0.1%) ⬇️ Carriedforward from e5bdab1
python 85.1% <100.0%> (+0.4%) ⬆️

*This pull request uses carry forward flags. Click here to find out more.

Files Coverage Δ
src/mqt/ddsim/job.py 68.0% <100.0%> (+0.6%) ⬆️
src/mqt/ddsim/qasmsimulator.py 98.9% <100.0%> (+0.1%) ⬆️
src/mqt/ddsim/unitarysimulator.py 87.8% <100.0%> (ø)

... and 1 file with indirect coverage changes

@andresbar98 andresbar98 changed the title Building DDSIM versions of QIskit Primitives Improved support of parametrized gates in DDSIM Backends Sep 13, 2023
@andresbar98 andresbar98 marked this pull request as ready for review September 14, 2023 11:50
@burgholzer burgholzer added enhancement Anything related to improvements of the existing library python Pull requests that update Python code labels Sep 17, 2023
Copy link
Member

@burgholzer burgholzer left a comment

Choose a reason for hiding this comment

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

Many thanks for the PR 👍🏼
I just went through it real quick and you can find some comments below.
Most importantly, I believe that some of the code duplication can be avoided.

src/mqt/ddsim/qasmsimulator.py Outdated Show resolved Hide resolved
src/mqt/ddsim/qasmsimulator.py Outdated Show resolved Hide resolved
src/mqt/ddsim/qasmsimulator.py Outdated Show resolved Hide resolved
src/mqt/ddsim/qasmsimulator.py Outdated Show resolved Hide resolved
src/mqt/ddsim/qasmsimulator.py Outdated Show resolved Hide resolved
test/python/simulator/test_qasm_simulator.py Outdated Show resolved Hide resolved
test/python/simulator/test_qasm_simulator.py Outdated Show resolved Hide resolved
test/python/simulator/test_qasm_simulator.py Outdated Show resolved Hide resolved
Copy link
Member

@burgholzer burgholzer left a comment

Choose a reason for hiding this comment

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

Hey 👋🏼
Just went over the changes once more. We are definitely converging.
I like that this is a way smaller change now than at the start of the PR.
You can find a couple of comments (mostly regarding typing) inline.
Additionally, the adjustments to the tests from the previous review are still open.

I also added you as a contributor to the repository, so you should now have the permissions to (re-)request reviews.

src/mqt/ddsim/qasmsimulator.py Outdated Show resolved Hide resolved
src/mqt/ddsim/qasmsimulator.py Outdated Show resolved Hide resolved
src/mqt/ddsim/qasmsimulator.py Outdated Show resolved Hide resolved
src/mqt/ddsim/qasmsimulator.py Outdated Show resolved Hide resolved
src/mqt/ddsim/qasmsimulator.py Outdated Show resolved Hide resolved
Copy link
Member

@burgholzer burgholzer left a comment

Choose a reason for hiding this comment

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

Thanks again, looking better now 👍🏼
Find some further comments inline.

src/mqt/ddsim/qasmsimulator.py Outdated Show resolved Hide resolved
src/mqt/ddsim/qasmsimulator.py Outdated Show resolved Hide resolved
src/mqt/ddsim/qasmsimulator.py Outdated Show resolved Hide resolved
src/mqt/ddsim/qasmsimulator.py Outdated Show resolved Hide resolved
src/mqt/ddsim/qasmsimulator.py Outdated Show resolved Hide resolved
src/mqt/ddsim/qasmsimulator.py Outdated Show resolved Hide resolved
test/python/simulator/test_qasm_simulator.py Show resolved Hide resolved
andresbar98 and others added 3 commits September 26, 2023 15:55
Modified error message

Co-authored-by: Lukas Burgholzer <[email protected]>
Modified error message

Co-authored-by: Lukas Burgholzer <[email protected]>
andresbar98 and others added 4 commits September 26, 2023 17:35
Signed-off-by: burgholzer <[email protected]>
use `assign_parameters` instead of the soon-to-be-deprecated `bind_parameters`.
Simplify input validation since many checks are already covered by the `assign_parameters` method in strict mode.

Signed-off-by: burgholzer <[email protected]>
Signed-off-by: burgholzer <[email protected]>
Copy link
Member

@burgholzer burgholzer left a comment

Choose a reason for hiding this comment

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

Thanks for the changes.
I took the liberty to fix the remaining errors and refactored parts of the code to be even simpler.
Most notably:

  • bind_parameters has been replaced by assign_aparameters since the former function is soon to be deprecated (see Deprecate bind_parameters in favor of assign_parameters Qiskit/qiskit#10792)
  • Several checks have been simplified as they are partly covered by the assign_parameters method internally or could be simplified by swapping some statements around
  • Typing of the parameters was simplified by introducing a type alias and using the more generic version from Qiskit

From my end, this looks good to go now. Let me know if you see anything problematic.

@burgholzer burgholzer added this to the Qiskit Primitives milestone Sep 27, 2023
@burgholzer burgholzer merged commit 127a41d into cda-tum:main Sep 28, 2023
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Anything related to improvements of the existing library python Pull requests that update Python code
Projects
Status: Done
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants