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

DDSIM version of Sampler primitive #300

Merged
merged 74 commits into from
Dec 5, 2023
Merged

DDSIM version of Sampler primitive #300

merged 74 commits into from
Dec 5, 2023

Conversation

andresbar98
Copy link
Contributor

@andresbar98 andresbar98 commented Sep 28, 2023

Creating a DDSIM version of one of the qiskit primitives, which is the "Sampler".
Code is inspired in the BackendSampler class from Qiskit https://qiskit.org/documentation/stubs/qiskit.primitives.BackendSampler.html, which in turn is a child class of the more general BaseSampler class https://qiskit.org/documentation/stubs/qiskit.primitives.BaseSampler.html .

  • Adapt BackendSampler class to our DDSIM backends (Only the ones that are compatible with this functionality)
  • Create suitable tests for this new feature
  • Document this feature

This is the second step towards solving issue #295

andresbar98 and others added 19 commits September 12, 2023 13:53
Modified error message

Co-authored-by: Lukas Burgholzer <burgholzer@me.com>
Modified error message

Co-authored-by: Lukas Burgholzer <burgholzer@me.com>
Co-authored-by: Lukas Burgholzer <burgholzer@me.com>
Signed-off-by: burgholzer <burgholzer@me.com>
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 <burgholzer@me.com>
Signed-off-by: burgholzer <burgholzer@me.com>
@codecov
Copy link

codecov bot commented Sep 28, 2023

Codecov Report

Merging #300 (cf53c9e) into main (7fd9287) will increase coverage by 0.1%.
The diff coverage is 100.0%.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##            main    #300     +/-   ##
=======================================
+ Coverage   92.5%   92.7%   +0.1%     
=======================================
  Files         32      34      +2     
  Lines       2540    2586     +46     
  Branches     349     349             
=======================================
+ Hits        2352    2398     +46     
  Misses       188     188             
Flag Coverage Δ *Carryforward flag
cpp 94.6% <ø> (ø) Carriedforward from 7fd9287
python 86.5% <100.0%> (+1.0%) ⬆️

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

Files Coverage Δ
src/mqt/ddsim/primitives/__init__.py 100.0% <100.0%> (ø)
src/mqt/ddsim/primitives/sampler.py 100.0% <100.0%> (ø)

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 again for the changes here. I believe this should be the last iteration before getting this merged.

Besides the inline comments: The docs site for the sampler is currently showing a warning for any circuit drawing. Could you please work around that by explicitly setting the drawing style as suggested by the warning? Please already use the newer style instead of the older fallback.

src/mqt/ddsim/sampler.py Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
test/python/test_sampler.py Outdated Show resolved Hide resolved
test/python/test_sampler.py Outdated Show resolved Hide resolved
test/python/test_sampler.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.

Some final touches on typing details, then this is ready to go in 👍🏼

test/python/test_sampler.py Outdated Show resolved Hide resolved
test/python/test_sampler.py Outdated Show resolved Hide resolved
src/mqt/ddsim/sampler.py Outdated Show resolved Hide resolved
src/mqt/ddsim/sampler.py Outdated Show resolved Hide resolved
src/mqt/ddsim/sampler.py Outdated Show resolved Hide resolved
src/mqt/ddsim/sampler.py Outdated Show resolved Hide resolved
src/mqt/ddsim/sampler.py Outdated Show resolved Hide resolved
burgholzer and others added 14 commits November 28, 2023 15:34
<!--pre-commit.ci start-->
updates:
- [github.com/pre-commit/mirrors-clang-format: v17.0.5 →
v17.0.6](pre-commit/mirrors-clang-format@v17.0.5...v17.0.6)
- [github.com/pre-commit/mirrors-prettier: v3.1.0 →
v4.0.0-alpha.3](pre-commit/mirrors-prettier@v3.1.0...v4.0.0-alpha.3)
<!--pre-commit.ci end-->

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Lukas Burgholzer <burgholzer@me.com>
Signed-off-by: burgholzer <burgholzer@me.com>
Signed-off-by: burgholzer <burgholzer@me.com>
Signed-off-by: burgholzer <burgholzer@me.com>
Signed-off-by: burgholzer <burgholzer@me.com>
Signed-off-by: burgholzer <burgholzer@me.com>
Signed-off-by: burgholzer <burgholzer@me.com>
Signed-off-by: burgholzer <burgholzer@me.com>
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.

Alright. I made a couple of final changes and applied some finishing touches, but this is ready to go in now. Many thanks! 🎉

@burgholzer burgholzer enabled auto-merge (squash) December 5, 2023 17:25
@burgholzer burgholzer merged commit b80e582 into cda-tum:main Dec 5, 2023
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request 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.

2 participants