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

♻️ BackendV2 migration of MQT DDSIM Qiskit backends #267

Merged
merged 115 commits into from
Sep 6, 2023
Merged

♻️ BackendV2 migration of MQT DDSIM Qiskit backends #267

merged 115 commits into from
Sep 6, 2023

Conversation

andresbar98
Copy link
Contributor

@andresbar98 andresbar98 commented Jul 24, 2023

Migrating all MQT simulators from BackendV1 to BackendV2.
Besides different access patterns I also eliminated some intermediate steps that involved dealing with QasmQobj and QasmQobjExperiment classes, since those are deprecated. Now we will be dealing with QuantumCircuit objects only.

@codecov
Copy link

codecov bot commented Jul 24, 2023

Codecov Report

Merging #267 (fd2dc2f) into main (cd2b8e1) will increase coverage by 0.7%.
The diff coverage is 96.3%.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##            main    #267     +/-   ##
=======================================
+ Coverage   91.7%   92.5%   +0.7%     
=======================================
  Files         31      32      +1     
  Lines       2475    2538     +63     
  Branches     351     351             
=======================================
+ Hits        2272    2349     +77     
+ Misses       203     189     -14     
Flag Coverage Δ *Carryforward flag
cpp 94.7% <ø> (ø) Carriedforward from cd2b8e1
python 84.7% <96.3%> (+4.7%) ⬆️

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

Files Changed Coverage Δ
src/mqt/ddsim/job.py 67.3% <81.8%> (-1.5%) ⬇️
src/mqt/ddsim/unitarysimulator.py 87.8% <86.0%> (+0.8%) ⬆️
src/mqt/ddsim/hybridqasmsimulator.py 89.8% <91.4%> (+1.5%) ⬆️
src/mqt/ddsim/qasmsimulator.py 98.8% <98.3%> (+3.4%) ⬆️
src/mqt/ddsim/header.py 100.0% <100.0%> (ø)
src/mqt/ddsim/hybridstatevectorsimulator.py 100.0% <100.0%> (ø)
src/mqt/ddsim/pathqasmsimulator.py 52.2% <100.0%> (-5.0%) ⬇️
src/mqt/ddsim/pathstatevectorsimulator.py 100.0% <100.0%> (ø)
src/mqt/ddsim/provider.py 95.0% <100.0%> (+5.0%) ⬆️
src/mqt/ddsim/statevectorsimulator.py 100.0% <100.0%> (ø)
... and 1 more

@hillmich
Copy link
Contributor

Thank you very much for creating the PR! Can you apply the changes directly to mqt/ddsim/qasmsimulator.py instead of creating a new file so that the history is correct?

@andresbar98
Copy link
Contributor Author

Hi
I just renamed the file back to qasmsimulator.py. Let me know if something else is missing

Copy link
Contributor

@hillmich hillmich 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 your quick response. It's a bit unfortunate that qiskit itself does not use BackendV2 yet in their standard backends, but they do have in in their fake backends.

Could take a look on into the above implementation and the migration guide to add the missing functions/members so the functionality remains the same and the tests pass?

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

@andresbar98 you might want to have a look at https://github.com/qiskit-community/qiskit-aqt-provider/tree/master
which uses the BackendV2 interface.

mqt/ddsim/qasmsimulator.py Fixed Show fixed Hide fixed
mqt/ddsim/qasmsimulator.py Fixed Show fixed Hide fixed
mqt/ddsim/qasmsimulator.py Fixed Show fixed Hide fixed
mqt/ddsim/qasmsimulator.py Fixed Show fixed Hide fixed
mqt/ddsim/qasmsimulator.py Fixed Show fixed Hide fixed
mqt/ddsim/qasmsimulator.py Fixed Show fixed Hide fixed
mqt/ddsim/qasmsimulator.py Fixed Show fixed Hide fixed
mqt/ddsim/qasmsimulator.py Fixed Show fixed Hide fixed
@andresbar98 andresbar98 changed the title BackendV2 migration of Qasmsimulator BackendV2 migration of Qasmsimulator and StatevectorSimulator Aug 2, 2023
mqt/ddsim/qasmsimulator.py Fixed Show fixed Hide fixed
mqt/ddsim/qasmsimulator.py Fixed Show fixed Hide fixed
mqt/ddsim/qasmsimulator.py Fixed Show fixed Hide fixed
mqt/ddsim/qasmsimulator.py Fixed Show fixed Hide fixed
mqt/ddsim/qasmsimulator.py Fixed Show fixed Hide fixed
mqt/ddsim/qasmsimulator.py Fixed Show fixed Hide fixed
@burgholzer
Copy link
Member

Hey @andresbar98 👋🏼
Many thanks for the changes. I just went through them and refactored the code a little further. The most notable changes:

  • The DDSIMHeader now directly inherits from the respective Qiskit object and is realized as a dataclass, which reduced some boilerplate code
  • All backends inheret from the QasmSimulatorBackend. This reduces large amounts of code duplication between the different backends.
  • Each backend class now has it's own target. During these changes, I noticed that you merely copy-pasted the target builder from the QASM simulator for the other targets. This did not reflect the original implementation where different backends only supported different basis gates
  • Simplified some expressions in the tests
  • I changed back the test that should actually fail due to the Qiskit bug so that it actually fails again here. This is nothing to be fixed in DDSIM, but in Qiskit. See the issue linked in one of the comments above. Although I believe there might be a workaround. I'll see what I can do.

Please have a look through these changes and see whether they make sense to you.
Any further improvements are appreciated.


def __init__(self) -> None:
super().__init__()
self.name = "hybrid_qasm_simulator"

Check warning

Code scanning / CodeQL

Overwriting attribute in super-class or sub-class

Assignment overwrites attribute name, which was previously defined in superclass [BackendV2](1).
def __init__(self) -> None:
super().__init__()
self.name = "hybrid_qasm_simulator"
self.description = ("MQT DDSIM Hybrid Schrodinger-Feynman simulator",)

Check warning

Code scanning / CodeQL

Overwriting attribute in super-class or sub-class

Assignment overwrites attribute description, which was previously defined in superclass [BackendV2](1).
super().__init__(configuration=configuration or BackendConfiguration.from_dict(conf), provider=provider)
def __init__(self) -> None:
super().__init__()
self.name = "hybrid_statevector_simulator"

Check warning

Code scanning / CodeQL

Overwriting attribute in super-class or sub-class

Assignment overwrites attribute name, which was previously defined in superclass [BackendV2](1). Assignment overwrites attribute name, which was previously defined in superclass [HybridQasmSimulatorBackend](2).
def __init__(self) -> None:
super().__init__()
self.name = "hybrid_statevector_simulator"
self.description = "MQT DDSIM Hybrid Schrodinger-Feynman Statevector simulator"

Check warning

Code scanning / CodeQL

Overwriting attribute in super-class or sub-class

Assignment overwrites attribute description, which was previously defined in superclass [BackendV2](1). Assignment overwrites attribute description, which was previously defined in superclass [HybridQasmSimulatorBackend](2).

def __init__(self) -> None:
super().__init__()
self.name = "path_sim_qasm_simulator"

Check warning

Code scanning / CodeQL

Overwriting attribute in super-class or sub-class

Assignment overwrites attribute name, which was previously defined in superclass [BackendV2](1).
def __init__(self) -> None:
super().__init__()
self.name = "path_sim_qasm_simulator"
self.description = "MQT DDSIM Simulation Path Framework"

Check warning

Code scanning / CodeQL

Overwriting attribute in super-class or sub-class

Assignment overwrites attribute description, which was previously defined in superclass [BackendV2](1).
super().__init__(configuration=configuration or BackendConfiguration.from_dict(conf), provider=provider)
def __init__(self) -> None:
super().__init__()
self.name = "statevector_simulator"

Check warning

Code scanning / CodeQL

Overwriting attribute in super-class or sub-class

Assignment overwrites attribute name, which was previously defined in superclass [BackendV2](1).
def __init__(self) -> None:
super().__init__()
self.name = "statevector_simulator"
self.description = "MQT DDSIM Statevector Simulator"

Check warning

Code scanning / CodeQL

Overwriting attribute in super-class or sub-class

Assignment overwrites attribute description, which was previously defined in superclass [BackendV2](1).

def __init__(self) -> None:
super().__init__()
self.name = "unitary_simulator"

Check warning

Code scanning / CodeQL

Overwriting attribute in super-class or sub-class

Assignment overwrites attribute name, which was previously defined in superclass [BackendV2](1).
def __init__(self) -> None:
super().__init__()
self.name = "unitary_simulator"
self.description = "MQT DDSIM Unitary Simulator"

Check warning

Code scanning / CodeQL

Overwriting attribute in super-class or sub-class

Assignment overwrites attribute description, which was previously defined in superclass [BackendV2](1).
Signed-off-by: burgholzer <burgholzer@me.com>
Signed-off-by: burgholzer <burgholzer@me.com>
Signed-off-by: burgholzer <burgholzer@me.com>
Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

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

CodeQL found more than 10 potential problems in the proposed changes. Check the Files changed tab for more details.

src/mqt/ddsim/target.py Outdated Show resolved Hide resolved
@burgholzer burgholzer linked an issue Sep 5, 2023 that may be closed by this pull request
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.

I think we finally have it. At least close to it. See the suggestions below. That should make the tests pass. Everything else can then be put into the issue you just created.

test/python/test_target.py Outdated Show resolved Hide resolved
test/python/test_target.py Outdated Show resolved Hide resolved
andresbar98 and others added 2 commits September 5, 2023 16:13
Co-authored-by: Lukas Burgholzer <burgholzer@me.com>
Co-authored-by: Lukas Burgholzer <burgholzer@me.com>
@burgholzer burgholzer linked an issue Sep 6, 2023 that may be closed by this pull request
@burgholzer burgholzer marked this pull request as ready for review September 6, 2023 06:14
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.

This LGTM now 👍🏼 Many thanks for this contribution!
Just waiting for CI to run through ✅

@burgholzer burgholzer enabled auto-merge (squash) September 6, 2023 06:20
@burgholzer burgholzer merged commit 0e27408 into cda-tum:main Sep 6, 2023
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 minor Part of a minor release python Pull requests that update Python code
Projects
Status: Done
Status: Done
Development

Successfully merging this pull request may close these issues.

Add Resets to Qiskit Backends that use the CircuitSimulator Migrate to Qiskit BackendV2
3 participants