embed_bqm silently assumes that chain_strength is a positive number

[pau557]

dwave-system/dwave/embedding/transforms.py

Lines 274 to 278 in b233941

	target_bqm.add_quadratic_from((p, q, -strength) for p, q in self.chain_edges(v))
	offset += strength * len(self._chain_edges[v])
	else: # if smear_vartype is dimod.BINARY
	target_bqm.add_variables_from((p, 2 * strength) for p in itertools.chain(*self.chain_edges(v)))
	target_bqm.add_quadratic_from((p, q, -4 * strength) for p, q in self.chain_edges(v))

embed_bqm assumes a positive sign for chain_strength:

import networkx as nx
from dwave.embedding import embed_bqm
import dimod

g = nx.Graph()
g.add_edge(100, 101)
bqm = dimod.BQM.from_ising({1: 0}, {})
emb = {1: [100, 101]}

for chain_strength in [-2, 2]:
    print(
        embed_bqm(bqm, embedding=emb, target_adjacency=g, chain_strength=chain_strength).quadratic
    )

Output:

{(101, 100): 2.0}
{(101, 100): -2.0}

The parameter is described as "coupling strength". I would expect that it either

1. Applies the sign passed (your sampler could be a maximizer)
2. Assumes minimization: It accepts any number and enforces the sign by taking abs of the input.

The current scheme where the sign is flipped is unexpected

[arcondello]

I think silently correcting it would be the more unexpected behavior. So IMO the current behavior, while slightly strange, is consistent and easy to explain. So if we were to make a change, it would be to raise an error.

[pau557]

In that case, the docstring should explain that the embedded chain coupler will be -1 * chain_strength

[arcondello]

Agreed. Want to make the PR?