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.

Sign up for GitHub

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

Jump to bottom

🧹 Noise-Aware Simulator Cleanup #491

Merged
merged 31 commits into from
Feb 5, 2024
Merged

🧹 Noise-Aware Simulator Cleanup #491

merged 31 commits into from
Feb 5, 2024

Conversation

33Gjl1Xe
Copy link
Contributor

@33Gjl1Xe 33Gjl1Xe commented Dec 1, 2023 •
edited
Loading

Description

This PR relates to this PR in the Simulator.

More specifically the PR refactors some functionality related to noise-aware quantum circuit simulation, i.e.:

  • I removed legacy functionality that allowed running the simulator in a less optimized version.
  • I added a function for collapsing measurements of density matrices.

Checklist:

  • The pull request only contains commits that are related to it.
  • I have added appropriate tests and documentation.
  • I have made sure that all CI jobs on GitHub pass.
  • The pull request introduces no new warnings and follows the project's style guidelines.

@33Gjl1Xe 33Gjl1Xe added enhancement New feature or request minor Minor version update c++ Anything related to C++ code python Anything related to Python code labels Dec 1, 2023
@codecov Codecov
Copy link

codecov bot commented Dec 1, 2023 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (9b22456) 91.2% compared to head (c71e92e) 91.2%.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##            main    #491     +/-   ##
=======================================
- Coverage   91.2%   91.2%   -0.1%     
=======================================
  Files        130     130             
  Lines      13709   13639     -70     
  Branches    2155    2144     -11     
=======================================
- Hits       12509   12442     -67     
+ Misses      1200    1197      -3
Flag Coverage Δ
cpp 90.9% <100.0%> (-0.1%) ⬇️
python 99.5% <ø> (ø)
Files Coverage Δ
include/mqt-core/dd/NoiseFunctionality.hpp 95.6% <100.0%> (-0.6%) ⬇️
include/mqt-core/dd/Package.hpp 96.2% <100.0%> (+0.1%) ⬆️

... and 2 files with indirect coverage changes

github-advanced-security[bot]
github-advanced-security bot found potential problems Dec 1, 2023
View reviewed changes
include/dd/Package.hpp Outdated
measOp = makeDDNode(static_cast<dd::Qubit>(0), identityMatrix);
}

for (dd::Qubit p = 1; p < numberOfQubits; p++) {

Check failure

Code scanning / CodeQL

Comparison of narrow type with wide type in loop condition

Comparison between [p](1) of type Qubit and [numberOfQubits](2) of wider type size_t.
@burgholzer
Copy link
Member

Hey 👋🏼
#444 just landed (finally). This simplified quite some code throughout the DD package.
Could you please rebase your changes on top of the current main branch?
Happy to take a look then!

@burgholzer burgholzer changed the title Noise-Aware Simulator Cleanup 🧹 Noise-Aware Simulator Cleanup Jan 31, 2024
@burgholzer burgholzer marked this pull request as ready for review January 31, 2024 12:02
@burgholzer burgholzer added the refactor Anything related to code refactoring label Jan 31, 2024
@burgholzer burgholzer added this to the DD Package Improvements milestone Jan 31, 2024
burgholzer
burgholzer requested changes Jan 31, 2024
View reviewed changes
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 is looking really good and makes the code quite a lot simpler. So, many thanks for that!
I just have a couple of very minor comments that should be pretty straight-forward to address. After that, this should be good to go!

@burgholzer
Copy link
Member

Ah, and before I forget it: could you please add an appropriate PR description here so that one can roughly understand what has been changed in this PR?
Once you pushed your changes and updated the description, this should be good to go!

@burgholzer
Copy link
Member

Note that the Python failures here are unrelated to your changes. They stem from the recent Qiskit update to 0.46.0.
I will quickly create a PR that fixes them.

@burgholzer
Copy link
Member

Note that the Python failures here are unrelated to your changes. They stem from the recent Qiskit update to 0.46.0. I will quickly create a PR that fixes them.

See #544. Once that is in, you should be able to merge that into your branch here and everything should be working again.

@33Gjl1Xe
Copy link
Contributor Author

33Gjl1Xe commented Feb 5, 2024

Note that the Python failures here are unrelated to your changes. They stem from the recent Qiskit update to 0.46.0. I will quickly create a PR that fixes them.

See #544. Once that is in, you should be able to merge that into your branch here and everything should be working again.

Thanks!

@33Gjl1Xe
Copy link
Contributor Author

33Gjl1Xe commented Feb 5, 2024

I believe the PR can now be merged.

burgholzer
burgholzer approved these changes Feb 5, 2024
View reviewed changes
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.

Yeah 🎉 thanks for all the effort. This is really looking great now.

@burgholzer burgholzer merged commit 3ee8bea into main Feb 5, 2024
33 checks passed
@burgholzer burgholzer deleted the nw_sim_features branch February 5, 2024 14:05
@burgholzer burgholzer mentioned this pull request Feb 14, 2024
Merged
4 tasks
burgholzer added a commit to cda-tum/mqt-ddsim that referenced this pull request Feb 14, 2024
@burgholzer
👽 adjust to latest mqt-core changes (#348)
a50b525 
## Description

This PR adjust the codebase to the latest changes from mqt-core
(specifically cda-tum/mqt-core#491).
Note that this merely applies the minimal changes necessary to run the
project with the latest version.
A proper cleanup of the noise-aware simulators is scheduled to arrive
with #321.

## Checklist:

<!---
This checklist serves as a reminder of a couple of things that ensure
your pull request will be merged swiftly.
-->

- [x] The pull request only contains commits that are related to it.
- [x] I have added appropriate tests and documentation.
- [x] I have made sure that all CI jobs on GitHub pass.
- [x] The pull request introduces no new warnings and follows the
project's style guidelines.

Signed-off-by: burgholzer <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@burgholzer burgholzer burgholzer approved these changes

Assignees
No one assigned
Labels
c++ Anything related to C++ code enhancement New feature or request minor Minor version update python Anything related to Python code refactor Anything related to code refactoring
Projects
MQT
Status: Done
MQT Core
Status: Done
MQT Simulation
Status: Done
Milestone
DD Package Improvements
Development

Successfully merging this pull request may close these issues.
2 participants
@33Gjl1Xe @burgholzer