Skip to content

Add lattice-utils to dwave-experimental - #44

Open
SebastianGitt wants to merge 18 commits into
dwavesystems:mainfrom
SebastianGitt:add-lattice-utils
Open

Add lattice-utils to dwave-experimental#44
SebastianGitt wants to merge 18 commits into
dwavesystems:mainfrom
SebastianGitt:add-lattice-utils

Conversation

@SebastianGitt

Copy link
Copy Markdown
Contributor

Add a minimal version of LatQA to dwave-experimental as a submodule named lattice-utils. Currently supports examples of a 1D Ising Chain and a 2D dimerized triangular lattice. Tests still need to be added to the pull request.

-move initialization of num_spins to subclass

@thisac thisac left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks @SebastianGitt! First pass of the lattice submodule.

  • I'd recommend running a formatter (black) since all of these files are new and there are a few places with incorrect spacing, etc.
  • I think the saving/loading of embeddings can be refined a bit, but I'll have a closer look at it later.

Comment thread dwave/experimental/lattice_utils/__init__.py Outdated
Comment thread dwave/experimental/lattice_utils/utils.py Outdated
Comment thread dwave/experimental/lattice_utils/utils.py Outdated
Comment thread dwave/experimental/lattice_utils/utils.py Outdated
Comment thread dwave/experimental/lattice_utils/utils.py Outdated
Comment thread dwave/experimental/lattice_utils/lattice/triangular.py Outdated
Comment thread dwave/experimental/lattice_utils/lattice/triangular.py
Comment thread dwave/experimental/lattice_utils/lattice/lattice.py Outdated
Comment thread dwave/experimental/lattice_utils/lattice/lattice.py
Comment thread dwave/experimental/lattice_utils/lattice/lattice.py
-added more comprehensive docstrings
-fixed formatting issues
-added more comprehensive docstrings
-fixed formatting issues
-removed spin reversal transform functionality
-uses automorphism module already in dwave-experimental
-uses data classes for experiment configs
-removed spin reversal transform functionality
-uses automorphism module already in dwave-experimental
-uses data classes for experiment configs
-formatted using black
@SebastianGitt
SebastianGitt marked this pull request as ready for review May 14, 2026 17:50

@andrew-d-king andrew-d-king left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Looks generally good; I am not able to pore over it in detail but it all seems sensible.

Comment thread dwave/experimental/lattice_utils/experiment/experiment.py
Comment thread dwave/experimental/lattice_utils/experiment/experiment.py
Comment thread dwave/experimental/lattice_utils/experiment/experiment.py
Comment thread examples/example_1D_Ising_chain.py Outdated
Comment thread examples/example_1D_Ising_chain.py Outdated
Comment thread examples/example_1D_Ising_chain.py Outdated
Comment thread examples/example_1D_Ising_chain.py Outdated
Comment thread examples/example_1D_Ising_chain.py Outdated
Comment thread examples/example_1D_Ising_chain.py Outdated
Comment thread examples/example_1D_Ising_chain.py Outdated

@thisac thisac left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Still have experiment module to look at. Re. the tests, it probably makes sense splitting them up into several files collected in a test_lattice_utils folder.

Comment thread dwave/experimental/lattice_utils/lattice/chain.py Outdated
Comment thread dwave/experimental/lattice_utils/lattice/lattice.py
Comment thread dwave/experimental/lattice_utils/lattice/lattice.py Outdated
Comment thread dwave/experimental/lattice_utils/lattice/lattice.py Outdated
Comment thread dwave/experimental/lattice_utils/lattice/embedded_lattice.py Outdated
Comment thread dwave/experimental/lattice_utils/observable/observable.py
Comment thread dwave/experimental/lattice_utils/observable/observable.py Outdated
Comment thread dwave/experimental/lattice_utils/observable/observable.py Outdated
Comment thread dwave/experimental/lattice_utils/observable/observable.py Outdated
Comment thread dwave/experimental/lattice_utils/observable/kinks.py Outdated
-added progress bars using TQDM
-added docstrings to the evaluation methods in observable.py
-misc cleanup
-added progress bars using TQDM
-added docstrings to the evaluation methods in observable.py
-misc cleanup

@jackraymond jackraymond left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice job, especially the integration of new automorphism functionality to make the shimming tutorial generalizable.

Some of this code is not lattice specific, I'd prefer it weren't bundled up in a lattice utility. Like embed_bqm (which mirrors a dwave.system) and unembed_bqm (which perhaps is, or should be dwave.system method), bootstrapping, bitstring compression. Ideally we could think how to extract it for more general use.

Examples don't run well, or at all for me. Adding an interface and some additional help messages on what is going on would be great. Perhaps cite papers etc where statistics are used.

There are a lot of magic numbers/methods floating around, if these could be promoted to function arguments and justified that would be great.

Some other naming conventions I found a bit confusing.

Comment thread dwave/experimental/lattice_utils/lattice/embedded_lattice.py
Comment thread dwave/experimental/lattice_utils/lattice/embedded_lattice.py Outdated
Comment thread dwave/experimental/lattice_utils/lattice/embedded_lattice.py
Comment thread dwave/experimental/lattice_utils/lattice/embedded_lattice.py
Comment thread dwave/experimental/lattice_utils/lattice/embedded_lattice.py
Comment thread dwave/experimental/lattice_utils/observable/observable.py
Comment thread dwave/experimental/lattice_utils/observable/observable.py Outdated
Comment thread examples/example_1D_Ising_chain.py Outdated
Comment thread examples/example_1D_Ising_chain.py Outdated
Comment thread examples/example_2D_geometric_frustration.py Outdated
-fix failing tests due to updated method signatures with explicit arguments
-rename energy_scale to signed_energy_scale
-remove existing examples and replaced with a new 1D chain shimming example
-restructure tests into separate files in a new folder
-update optimize function to accept custom dimod solvers
-expand docstring for ``embed_bqm`` method to explain difference from existing methods in dwave-system
-promote ``optimize`` and ``make_networkx_graph`` to public methods

@thisac thisac left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looking good so far! A couple of comments and improvements before ready to merge.

One thing that we discussed already: tqdm should be made into an optional dependency. Currently it's required to install.

Comment thread dwave/experimental/lattice_utils/experiment/experiment.py Outdated
Comment thread dwave/experimental/lattice_utils/experiment/experiment.py Outdated
Comment thread dwave/experimental/lattice_utils/experiment/experiment.py Outdated
Comment thread dwave/experimental/lattice_utils/experiment/experiment.py Outdated
Comment thread dwave/experimental/lattice_utils/experiment/experiment.py Outdated
Comment thread dwave/experimental/lattice_utils/observable/observable.py Outdated
Comment thread dwave/experimental/lattice_utils/observable/observable.py Outdated
Comment thread dwave/experimental/lattice_utils/observable/observable.py
Comment thread dwave/experimental/lattice_utils/observable/observable.py Outdated
Comment thread releasenotes/notes/add-lattice-utils-49e5efd79268e69d.yaml

@thisac thisac left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

A few more comments...

Comment thread dwave/experimental/lattice_utils/experiment/experiment.py Outdated
Comment thread dwave/experimental/lattice_utils/lattice/chain.py Outdated
Comment thread dwave/experimental/lattice_utils/lattice/lattice.py Outdated
# See the License for the specific language governing permissions and
# limitations under the License.

"""Shimming example for 1D Ising chain."""

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Let's also rename this file to just 1D_Ising_chain_shim.py or maybe prefix it with lattice_utils or even put it in a lattice_utils subfolder since there are quite a few more lattice-utils examples incoming, no?

Comment thread examples/example_1D_Ising_chain_shim.py Outdated
Comment thread examples/example_1D_Ising_chain_shim.py Outdated
Comment thread dwave/experimental/lattice_utils/experiment/experiment.py Outdated
-make TQDM an optional import
-order imports
-add some missing docstrings
-removed subscript unpacking for Python 3.10 compatibility
-updated licenses to 2026
@codecov-commenter

codecov-commenter commented Jul 21, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 89.96317% with 109 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.46%. Comparing base (703b3cd) to head (3f092e2).
⚠️ Report is 28 commits behind head on main.

Files with missing lines Patch % Lines
...xperimental/lattice_utils/experiment/experiment.py 83.58% 65 Missing ⚠️
...wave/experimental/lattice_utils/lattice/lattice.py 89.62% 14 Missing ⚠️
...xperimental/lattice_utils/observable/observable.py 90.00% 10 Missing ⚠️
...e/experimental/lattice_utils/lattice/triangular.py 91.95% 7 Missing ⚠️
...xperimental/lattice_utils/observable/triangular.py 82.75% 5 Missing ⚠️
...ave/experimental/lattice_utils/lattice/optimize.py 94.44% 3 Missing ⚠️
...rimental/lattice_utils/lattice/embedded_lattice.py 97.75% 2 Missing ⚠️
dwave/experimental/lattice_utils/lattice/chain.py 95.00% 1 Missing ⚠️
...ave/experimental/lattice_utils/observable/kinks.py 96.00% 1 Missing ⚠️
dwave/experimental/lattice_utils/utils.py 96.29% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #44      +/-   ##
==========================================
- Coverage   95.38%   92.46%   -2.92%     
==========================================
  Files          13       30      +17     
  Lines         931     2017    +1086     
==========================================
+ Hits          888     1865     +977     
- Misses         43      152     +109     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@thisac thisac left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

A couple of things noticed, but otherwise happy to merge when approved by @andrew-d-king.

Comment thread dwave/experimental/lattice_utils/lattice/triangular.py
Comment thread dwave/experimental/lattice_utils/observable/kinks.py
Comment thread dwave/experimental/lattice_utils/lattice/optimize.py
Comment thread dwave/experimental/lattice_utils/experiment/experiment.py Outdated
Comment thread dwave/experimental/lattice_utils/experiment/experiment.py
Comment thread dwave/experimental/lattice_utils/experiment/samplercall.py Outdated
Comment thread dwave/experimental/lattice_utils/lattice/lattice.py Outdated
Comment thread dwave/experimental/lattice_utils/lattice/lattice.py Outdated
Comment thread dwave/experimental/lattice_utils/lattice/lattice.py Outdated
Comment thread dwave/experimental/lattice_utils/lattice/triangular.py Outdated
-import Experiment and Lattice types for proper type hints
-fix typos
-SamplerCall is now a proper data class

@randomir randomir left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

A few comments, after a first pass over the experiment and lattice modules.

Comment thread examples/mca_shim_AO_FB.py Outdated
Comment on lines +28 to +33
from pathlib import Path
import sys
REPO_ROOT = Path(__file__).resolve().parents[1]
if str(REPO_ROOT) not in sys.path:
sys.path.insert(0, str(REPO_ROOT))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is not needed when example requirements are installed. Note that those requirements include the dwave-experimental package in the parent repo (-e .. line).

Suggested change
from pathlib import Path
import sys
REPO_ROOT = Path(__file__).resolve().parents[1]
if str(REPO_ROOT) not in sys.path:
sys.path.insert(0, str(REPO_ROOT))

)


def _make_mock_sampler(

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why is dwave.system.testing.MockDWaveSampler not sufficient here? Could we rather extend the existing mock sampler if some functionality is needed here?

if sampler is None:
return self._get_path(kind)

if type(sampler).__name__ == "MockDWaveSampler":

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

why not isinstance test here?

__all__ = ['optimize', 'ExponentialBackoffSimulatedAnnealingSampler']


class ExponentialBackoffSimulatedAnnealingSampler(dimod.Sampler):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It's not backoff (timeout) that's exponential, but number of sweeps.

Suggested change
class ExponentialBackoffSimulatedAnnealingSampler(dimod.Sampler):
class ExponentialSweepsSimulatedAnnealingSampler(dimod.Sampler):

reference_sample = None

# If the lattice is embedded, we should optimize the logical lattice
if hasattr(lattice, "logical_lattice"):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why do you prefer duck typing over explicit type check?

Suggested change
if hasattr(lattice, "logical_lattice"):
if isinstance(lattice, EmbeddedLattice):

Here, and elsewhere.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

In this case it was to avoid circular imports

def __init__(
self,
*,
inst: Lattice,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Better name:

Suggested change
inst: Lattice,
lattice: Lattice,

if result_fields is None:
result_fields = list(data.keys())
if ignore_shim:
result_fields.remove("shim_data")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This fails with ValueError when "shim_data" is not in the list -- e.g. on second file (!), or when passed-in arg is not None, but omits the shim data value.


# Strip spaces and replace other unswanted symbols with underscores.
pathstring = pathstring.replace(" ", "_")
for bad_symbol in ":;,":

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Here, and elsewhere (security): use an allowlist for pathname construction, not a short denylist.

E.g. allow only [A-Za-z0-9._-], replace the rest with _.

Consider anneal_schedule or solver name containing /../../.


try:
with lzma.open(filename, "rb") as f:
data = pickle.load(f)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The use of pickle (in a loop) is the biggest security/safety issue I see in this PR.

We can allow it for now in the experimental package, but we will have to replace it with np.savez/np.savez_compressed/orjson/JSON/similar serialization before moving this code to Ocean.

Unless you feel keen on rewriting the serialization in this PR, please add a warning to the experiment/lattice module docs.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I'll add the warning. It will be deprecated in favour of using databases before its merged to Ocean.

else:
break

best_sampleset.info["num_sweeps_exit"] = num_sweeps

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This fails if the while loop doesn't run!

-use an allowlist for pathname construction
-add warning about the security risk of using pickled data
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants