Feature/mock d wave sampler

[jackraymond]

I updated the interface - sampleset format, parameters and properties to match DWaveSampler() on Advantage processors. I swapped out the neal sampler for a greedy sampler as the QPU sampler substitute. This is more efficient, and produces a better controlled set of low energy samples (easier to work with for testing). The mock_sampler can now handle the 'max_answers', 'answer_mode' and 'initial_state' input parameters in a sensible way. I also added argument checking and warnings for the unmocked parameters, warnings can be switched off at initialization time. I removed the special handling of flux_biases, this seemed to be tied rather arbitrarily to the substitute sampler defaults - which should not be considered forward compatible. Tests for virtualGraphComposite() should be reworked if they rely on the distribution.

[codecov-commenter]

Codecov Report

Merging #443 (97b64ec) into master (7bf16fc) will decrease coverage by 3.84%. The diff coverage is 82.95%.

@@            Coverage Diff             @@
##           master     #443      +/-   ##
==========================================
- Coverage   90.01%   86.17%   -3.85%     
==========================================
  Files          23       23              
  Lines        1533     1555      +22     
==========================================
- Hits         1380     1340      -40     
- Misses        153      215      +62     

Impacted Files	Coverage Δ
dwave/system/samplers/dwave_sampler.py	83.13% <55.55%> (-4.67%)	:arrow_down:
dwave/system/testing.py	90.90% <86.07%> (-4.01%)	:arrow_down:
dwave/system/samplers/leap_hybrid_sampler.py	61.42% <0.00%> (-14.29%)	:arrow_down:
dwave/system/samplers/clique.py	74.35% <0.00%> (-10.26%)	:arrow_down:
dwave/system/composites/embedding.py	96.57% <0.00%> (-0.58%)	:arrow_down:

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

[pau557]

Great work.

This is missing the topology-specific to_networkx_graph, see https://github.com/dwavesystems/dwave-system/blob/4fa4b27a422a63afa65e90773ed4cf4d918c2f9f/dwave/system/samplers/dwave_sampler.py#L486 (ie. getting a dnx.chimera_graph with all the topology info instead of a simple nx.graph)

You could just inherit that by subclassing DWaveSampler instead of Sampler at the top.

[arcondello]

Agree that inheriting may make sense, but be careful if you do - it's important that you not accidental require a connection to SAPI.

[pau557]

Agree with Alex about the caution, but I think that the methods you're already overwriting will take care of that

[jackraymond]

I'm pushing some broken code to show what happens when I try to inherit from DWaveSampler(), indeed the API request comes along for the ride. Maybe someone will have a solution. If not I'll copy to_networkx_graph() in, and revert to the old class inheritance. Code is broken in the sense that it now requires SAPI connection to run. I suppose this means it's an even better Mock, but might not be ideal for some users. Would be nice to be able to run without an internet connection. Perhaps an option can be added for DWaveSampler() class for running offline (in so far as that is possible by default).

[randomir]

I'm pushing some broken code to show what happens when I try to inherit from DWaveSampler(), indeed the API request comes along for the ride. Maybe someone will have a solution. If not I'll copy to_networkx_graph() in, and revert to the old class inheritance. Code is broken in the sense that it now requires SAPI connection to run. I suppose this means it's an even better Mock, but might not be ideal for some users. Would be nice to be able to run without an internet connection. Perhaps an option can be added for DWaveSampler() class for running offline (in so far as that is possible by default).

I don't see your mock sampler instantiating the cloud client anywhere, so you managed to suppress the unwanted network activity. :electric_plug: :x:

But I would still advise against subclassing DWaveSampler here. First, I'm not sure I see it as semantically correct (given you're implementing only "functional" aspect here -- dimod.Sampler/dimod.Structured interface, ignoring the "network" aspect -- client and solver attributes of DWaveSampler). Second, you need to override practically all methods and attributes to suppress the API requests, including the failover mechanism (that adds complexity for functionality not wanted here), leaving you only with to_networkx_graph() you're reusing from the superclass. This creates future-fragile code that needs to be updated in sync with the superclass. Coincidentally, to_networkx_graph is actually implemented in dimod.Structured, and you get it for free if you define self.nodelist and self.edgelist. Except you're using DWaveSampler.to_networkx_graph() in "reverse" to create a graph based on topology by setting nodelist and edgelist to None (internal dnx logic).

Long term, I would rather use cloud client's solver data mocks and mock the network aspect as well (provide functional, if mocked, .client and .solver attrs). See #283 and https://github.com/dwavesystems/dwave-cloud-client/issues/387.

But that's a larger effort involving creation of dwave.cloud.client.MockSolver and MockClient. Short term (this PR) I recommend reverting to dimod superclass(es) and manual graph creation perhaps via an utility function in the testing module.

[jackraymond]

I realized after the fact the reason the code hung offline is probably that DWaveSampler() was called in the integration testing, so your observation on the cloud client is I assume correct. Code was not broken in the sense I thought. I think the suggestion to use the DWaveSampler() inheritance was in order to correctly inherit the topology-specific class information (dnx), which is not present in dimod.to_networkx_graph() . In any case, I'll take your recommendation to revert to dimod.Structured and copy in a local helper function again.

[davidmerwin]

❤️

[JoelPasvolsky]

What is the plan for to_networkx_graph in the end? I just wasted a lot of time on that differnce: I added an example that doctest tests using the mock DWaveSampler and includes the lines:

qpu = DWaveSampler()
embedding = find_clique_embedding(bqm.variables, qpu.to_networkx_graph())

The find_clique_embedding balks because ValueError: input graph must either be a dwave_networkx.pegasus_graph, dwave_networkx.chimera_graph or dwave_networkx.zephyr_graph on the qpu.to_networkx_graph() in the mocked tests, though of course it works nicely with the real solver so it takes a while to figure out why the failure.

[jackraymond]

What is the plan for to_networkx_graph in the end? I just wasted a lot of time on that differnce: I added an example that doctest tests using the mock DWaveSampler and includes the lines:

qpu = DWaveSampler()
embedding = find_clique_embedding(bqm.variables, qpu.to_networkx_graph())

The find_clique_embedding balks because ValueError: input graph must either be a dwave_networkx.pegasus_graph, dwave_networkx.chimera_graph or dwave_networkx.zephyr_graph on the qpu.to_networkx_graph() in the mocked tests, though of course it works nicely with the real solver so it takes a while to figure out why the failure.

This pull request solves this problem. to_networkx_graph() contains dwave_networkx specific information. The problem arises because to_networkx_graph exists in the dimod.Structured class in a form that does not propagate lattice specific topology information. I initially solved this at @pau557 suggestion by inheriting from DWaveSampler, but per @randomir I went back to the old inheritance structure and simply copied over the function.

[jackraymond]

Left as to do list (last commit): Rework to ensure contradictions between nodelist, edgelist and properties are impossible (see @randomir closed comment). Under forseeable use cases, I think contradictions will not arise so this is good enough.

[jackraymond]

What is the plan for to_networkx_graph in the end? I just wasted a lot of time on that differnce: I added an example that doctest tests using the mock DWaveSampler and includes the lines:

qpu = DWaveSampler()
embedding = find_clique_embedding(bqm.variables, qpu.to_networkx_graph())

The find_clique_embedding balks because ValueError: input graph must either be a dwave_networkx.pegasus_graph, dwave_networkx.chimera_graph or dwave_networkx.zephyr_graph on the qpu.to_networkx_graph() in the mocked tests, though of course it works nicely with the real solver so it takes a while to figure out why the failure.

The mock sampler now supports dwave_networkx graph generation.