Repository: dwavesystems/dimod

Issue:
Don't change vartype unnecessarily in Sampler.sample
**Application**
The dimod `Sampler` abc allows the user to implement one of `.sample`, `.sample_ising`, or `.sample_qubo` for it to work. It does this by using a figure-8 loop of sample calls, see:
https://github.com/dwavesystems/dimod/blob/378b03a1df067ae4c12249bfcebf939a5324f1c6/dimod/core/sampler.py#L135
However, this means that we're changing the vartype of the BQM always when `.sample` is not implemented. This is slow and violates the principle of least astonishment.

**Proposed Solution**

The way the `samplemixinmethod` decorator is implemented means we could 'target' the implemented sample method.



Discussion:

Patch:
diff --git a/dimod/core/sampler.py b/dimod/core/sampler.py
index 19a8d82f..6df4f0b8 100644
--- a/dimod/core/sampler.py
+++ b/dimod/core/sampler.py
@@ -135,16 +135,32 @@ class Sampler:
     def sample(self, bqm, **parameters):
         """Samples from a binary quadratic model using an implemented sample method.
         """
+
+        # we try to use the matching sample method if possible
         if bqm.vartype is Vartype.SPIN:
-            Q, offset = bqm.to_qubo()
-            response = self.sample_qubo(Q, **parameters)
-            response.change_vartype(Vartype.SPIN, energy_offset=offset)
-            return response
+            if not getattr(self.sample_ising, '__issamplemixin__', False):
+                # sample_ising is implemented
+                h, J, offset = bqm.to_ising()
+                sampleset = self.sample_ising(h, J, **parameters)
+                sampleset.record.energy += offset
+                return sampleset
+            else:
+                Q, offset = bqm.to_qubo()
+                sampleset = self.sample_qubo(Q, **parameters)
+                sampleset.change_vartype(Vartype.SPIN, energy_offset=offset)
+                return sampleset
         elif bqm.vartype is Vartype.BINARY:
-            h, J, offset = bqm.to_ising()
-            response = self.sample_ising(h, J, **parameters)
-            response.change_vartype(Vartype.BINARY, energy_offset=offset)
-            return response
+            if not getattr(self.sample_qubo, '__issamplemixin__', False):
+                # sample_qubo is implemented
+                Q, offset = bqm.to_qubo()
+                sampleset = self.sample_qubo(Q, **parameters)
+                sampleset.record.energy += offset
+                return sampleset
+            else:
+                h, J, offset = bqm.to_ising()
+                sampleset = self.sample_ising(h, J, **parameters)
+                sampleset.change_vartype(Vartype.BINARY, energy_offset=offset)
+                return sampleset
         else:
             raise RuntimeError("binary quadratic model has an unknown vartype")
 
@@ -153,13 +169,11 @@ class Sampler:
         """Samples from an Ising model using an implemented sample method.
         """
         bqm = BinaryQuadraticModel.from_ising(h, J)
-        response = self.sample(bqm, **parameters)
-        return response
+        return self.sample(bqm, **parameters)
 
     @samplemixinmethod
     def sample_qubo(self, Q, **parameters):
         """Samples from a QUBO using an implemented sample method.
         """
         bqm = BinaryQuadraticModel.from_qubo(Q)
-        response = self.sample(bqm, **parameters)
-        return response
+        return self.sample(bqm, **parameters)


Test patch:
diff --git a/tests/test_sampler.py b/tests/test_sampler.py
index c3c64418..ce3692cd 100644
--- a/tests/test_sampler.py
+++ b/tests/test_sampler.py
@@ -264,3 +264,51 @@ class TestSamplerClass(unittest.TestCase):
         expected_resp = dimod.Response.from_samples([[1]], {"energy": [-3]}, {}, dimod.BINARY)
         np.testing.assert_almost_equal(resp.record.sample, expected_resp.record.sample)
         np.testing.assert_almost_equal(resp.record.energy, expected_resp.record.energy)
+
+    def test_spin_bqm_to_sample_ising(self):
+
+        class CountBQM(dimod.BinaryQuadraticModel):
+            # should never have vartype changed
+            def change_vartype(self, *args, **kwargs):
+                raise RuntimeError
+
+            def to_qubo(self):
+                raise RuntimeError
+
+        class Ising(dimod.Sampler):
+            parameters = None
+            properties = None
+
+            def sample_ising(self, h, J):
+                bqm = dimod.BinaryQuadraticModel.from_ising(h, J)
+                samples = [1]*len(bqm)
+                return dimod.SampleSet.from_samples_bqm(samples, bqm)
+
+        sampler = Ising()
+        cbqm = CountBQM.from_ising({0: -3}, {(0, 1): -1.5}, offset=1.3)
+        sampleset = sampler.sample(cbqm)
+        dimod.testing.assert_response_energies(sampleset, cbqm)
+
+    def test_binary_bqm_to_sample_qubo(self):
+
+        class CountBQM(dimod.BinaryQuadraticModel):
+            # should never have vartype changed
+            def change_vartype(self, *args, **kwargs):
+                raise RuntimeError
+
+            def to_ising(self):
+                raise RuntimeError
+
+        class Qubo(dimod.Sampler):
+            parameters = None
+            properties = None
+
+            def sample_qubo(self, Q):
+                bqm = dimod.BinaryQuadraticModel.from_qubo(Q)
+                samples = [1]*len(bqm)
+                return dimod.SampleSet.from_samples_bqm(samples, bqm)
+
+        sampler = Qubo()
+        cbqm = CountBQM.from_qubo({(0, 0): -3, (1, 0): 1.5}, offset=.5)
+        sampleset = sampler.sample(cbqm)
+        dimod.testing.assert_response_energies(sampleset, cbqm)