dwavesystems / dimod

A shared API for QUBO/Ising samplers.
https://docs.ocean.dwavesys.com/en/stable/docs_dimod/
Apache License 2.0
121 stars 80 forks source link

Add ability to merge serveral DQM or BQM objects #785

Open arcondello opened 3 years ago

arcondello commented 3 years ago

It would be useful to merge several BQM/DQMs.

At the moment one can do

bqm.update(other_bqm)

to merge two BQM objects.

We should add a DQM.update as well.

Further, it would be useful to combine many DQM/BQM together at relative weights. See https://github.com/dwavesystems/dimod/pull/784#issuecomment-769358918

One way to accomplish this would be a merge function, as suggested in https://github.com/dwavesystems/dimod/pull/784#issuecomment-769358918

Another would be to add a weight parameter to the BQM.update() and DQM.update() methods. Likely the function would end up calling the methods, but rolling them together would be nice.

hsadeghidw commented 3 years ago

This could either use a function or operator overloading of __add__ and __radd__

new_bqm = sum(bqms)

arcondello commented 3 years ago

Agree. Or even new = bqm + 2 * other_bqm

Could even allow bqm ** 2 when there are no quadratic biases...

hsadeghidw commented 3 years ago

I have been thinking about this. I believe that the operations (+, -, *, pow(), sum()) should always return a new object.

For example: bqm + bqm + bqm

can be unambiguous, if bqm changes in the first operation. For example, if the bqm only has a bqm.offset = 1, the expression above can result in bqm.offset being 4 instead of 3.

arcondello commented 3 years ago

Yes, of course. + should always make a new one, += modifies an existing. Did I imply otherwise?

hsadeghidw commented 3 years ago

nope. I had the wrong starting point in my head. To save space, one might consider modifying the largest BQM rather than creating a new one.

arcondello commented 3 years ago

Makes sense, I think merge would look something like

def merge(bqms):
    merged_bqm = bqms[0].copy()
    for bqm in bqms[1:]:
        merged_bqm += bqm
    return merged_bqm

Obviously with input checking and avoiding the copy on list slice.

hsadeghidw commented 3 years ago

In 0.10.x there are two BQM objects, AdjVectorBQM and AdjDictBQM. right? Should all methods add to both objects as well as DQM?

All in one PR or multiple PR?

arcondello commented 3 years ago

Unless it's urgent, I would not make any PRs on this. There are some fundamental decisions about structure and API that need to be worked out. In the case that we need performant merging urgently, I'd like to address https://github.com/dwavesystems/dimod/issues/757. It's not a hard change to make, but given the current implementation work, I should do it so we're not doing redundant work.

Likely I can easily fold this into the 0.10.x release.