-
Notifications
You must be signed in to change notification settings - Fork 15
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
Feature/path integral simulated annealing #72
base: main
Are you sure you want to change the base?
Feature/path integral simulated annealing #72
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did a quick sanity review, will do a deeper one soon
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another relatively quick pass. Most comments are stylistic. I assume the C++ code has been in use for a while so I am inclined to not make structural changes. That said, can we run it through clang-format if we haven't already? And are there and C++ tests we can add?
// add thread_local (C++11) support for MSVC 9 and 10 (py2.7 and py3.4 on win) | ||
// note: thread_local support was added in MSVC 14 (Visual Studio 2015, MSC_VER 1900) | ||
#if _MSC_VER < 1900 | ||
#define thread_local __declspec(thread) | ||
#endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
based on the comment, I don't think we need this anymore. Don't remove it from elsewhere (it'll make a long review even longer 😄), but we don't need to add it here.
I'm going to be using this code, I'll check the documentation is comprehensible! |
…ove unnecessary files. Not done: google-style docstrings and namespace all C++ code.
…ove unnecessary files. Not done: google-style docstrings and namespace all C++ code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the doc errors are unrelated to this PR. I can push a fix and then we can rebase this one.
@@ -1,4 +1,4 @@ | |||
# Copyright 2018 D-Wave Systems Inc. | |||
# Copyright 2024 D-Wave |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like there are no functional changes in this file. I would rather formatting changes happen as part of another PR. But certainly the copyright date doesn't need to be updated.
@@ -1,4 +1,4 @@ | |||
# Copyright 2018 D-Wave Systems Inc. | |||
# Copyright 2024 D-Wave |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like there are no functional changes in this file. I would rather formatting changes happen as part of another PR. But certainly the copyright date doesn't need to be updated.
@@ -0,0 +1,153 @@ | |||
/* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar to the comment on the README, I don't think we want this file in the package. It will still be in the other repo though
features: | ||
- Add ``PathIntegralAnnealingSampler``, a SimulatedAnnealingSampler style wrapper for the dwave-pimc repository. This supports simulation of path-integral dynamics and equilibrium sampling of quantum Boltzmann distributions: https://doi.org/10.1038/s41467-021-20901-5 | ||
- Add ``RotorModelAnnealingSampler``, a SimulatedAnnealingSampler style code for rotor model simulation. This supports a standard classical algorithm for approximation of spin dynamics: https://doi.org/10.48550/arXiv.1401.7087 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I was wrong about the docs failures. It is this PR, specifically it doesn't like the :
in the release notes.
---
features:
- Add ``PathIntegralAnnealingSampler``, a SimulatedAnnealingSampler style wrapper for the dwave-pimc repository. This supports simulation of path-integral dynamics and equilibrium sampling of quantum Boltzmann distributions (`King et al., 2021 <https://www.nature.com/articles/s41467-021-20901-5>`_).
- Add ``RotorModelAnnealingSampler``, a SimulatedAnnealingSampler style code for rotor model simulation. This supports a standard classical algorithm for approximation of spin dynamics (`Shin et al., 2014 <https://doi.org/10.48550/arXiv.1401.7087>`_).
Thanks @JoelPasvolsky for the help with this.
Add path-integral and rotor-model dimod.Samplers(). The former is a wrapper for https://github.com/dwavesystems/dwave-pimc; the latter is new code.
At the python-wrapper level code is structured to align closely with SimulatedAnnealingSampler. Under default operation the main difference is the 'classical state' on which simulated annealing is performed: binary states (
SimulatedAnnealingSampler
), [discretized-]angles (RotorModelAnnealingSampler
), or paths (PathIntegralAnnealingSampler
, aliasSimulatedQuantumAnnealingSampler
); These different states require different local update methods, and the schedule is generalized to allow variation of a transverse field in the latter cases, but otherwise the algorithms are very similar.These samplers have been carefully studied over many years as emulators of quantum dynamics and as optimization heuristics: https://doi.org/10.1038/s41467-021-20901-5