dimod icon indicating copy to clipboard operation
dimod copied to clipboard

BQM.as_vartype and SampleSet.as_vartype or similar

Open arcondello opened this issue 5 years ago • 5 comments

BQM.change_vartype and SampleSet.change_vartype either always copy or always act in place. It would be useful to be able to cast the BQM/SampleSet to an appropriate vartype, copying only in the case that they don't match. It would be similar behavior to

if bqm.vartype is dimod.SPIN:
    spin_bqm = bqm
else:
    spin_bqm = bqm.change_vartype(dimod.SPIN, inplace=False)

This could be accomplished with a new method

bqm = bqm.as_vartype(dimod.SPIN, copy=False)

as inspired by ndarray.astype. But this is fairly redundant with the existing method and it might be confused with the existing as_vartype function.

An alternative would be adding a new keyword argument to .change_vartype, e.g.

bqm = bqm.change_vartype(dimod.SPIN, inplace=False, copy=False)

though I think copy is a pretty confusing name.

arcondello avatar Aug 19 '20 21:08 arcondello

Perhaps this will only confuse matters -- this is related to @hhtong's latest comment on dwavesystems/dwave-system#321 -- I've hacked up a view whose vartype can change without making copies

class EitherView(VartypeView):
    def __init__(self, bqm, vartype = None):
        self._base_bqm = bqm

        if bqm.vartype is SPIN:
            self._which = {dimod.SPIN: bqm,
                           dimod.BINARY: BinaryView(bqm)}
        else:
            self._which = {dimod.SPIN: SpinView(bqm),
                           dimod.BINARY: bqm}

        if vartype is None:
            self.vartype = bqm.vartype
        else:
            self.vartype = vartype

    @property
    def _bqm(self):
        return self._which[self._vartype]

    def change_vartype(self, vartype, inplace=True):
        if inplace:
            self._vartype = vartype
        else:
            return EitherView(self._base_bqm, vartype)


    @property
    def binary(self):
        return EitherView(self._base_bqm, BINARY)

    @property
    def spin(self):
        return EitherView(self._base_bqm, SPIN)

    @property
    def offset(self):
        return self._bqm.offset

    @offset.setter(self, bias):
    def offset
        self._bqm.offset = bias

    def copy(self):
        if self._vartype is self._base_bqm.vartype:
            return self._base_bqm.copy()
        else:
            return self._base_bqm.change_vartype(self._vartype, inplace=False)

    def get_linear(self, v):
        return self._bqm.get_linear(v)

    def get_quadratic(self, u, v, default=None):
        return self._bqm.get_quadratic(u, v, default=default)

    def set_linear(self, v, bias):
        self._bqm.set_linear(self, v, bias)

    def set_quadratic(self, u, v, bias):
        self._bqm.set_quadratic(self, u, v, bias)

boothby avatar Aug 21 '20 15:08 boothby

One feature of the above is that you can change the vartype inplace without making a copy, and then if you want to make a copy, you can simply call .copy(). I attempted to embue this with an as_vartype(self, inplace, copy) as you suggested, but found that inplace=True and copy=True is somewhat nonsensical. It may be better to return an EitherView to support bqm.spin or bqm.binary, which supports changing vartype inplace, and then if the user wants a concrete copy, they can call .copy()

boothby avatar Aug 21 '20 15:08 boothby

~This implementation has some issues, specifically that set_quadratic etc do not respect the vartype of the BQM~ (edit: I see the ._bqm property now). The existing .SpinView and .BinaryView are already intended to cover the "no copy" case, i.e. just by doing bqm = bqm.binary if BINARY is the desired vartype. The problem @hhtong ran into in https://github.com/dwavesystems/dwave-system/pull/321#discussion_r473357208 came about because .change_vartype is not defined for the vartype views. Another way to address that issue would be to update .change_vartype to work in the inplace=False case.

arcondello avatar Aug 21 '20 15:08 arcondello

We could combine the EitherView with the existing VartypeView and use that instead of the two subclasses, which would make change_vartype well defined with fewer overall layers of abstraction.

arcondello avatar Aug 21 '20 15:08 arcondello

Ooh I quite like that

boothby avatar Aug 21 '20 15:08 boothby