Addressing issue #1364: option for a variable check in add_discrete method

[aivarsoo]

As discussed in the issue #1364, the variable check in ConstrainedQuadraticModel.add_discrete method can be skipped in some cases and the user can decide if this is truly necessary.

This is implemented by adding a do_var_check boolean variable (default: True), which enables the check if it's true, in the methods ConstrainedQuadraticModel.add_discrete_from_model, ConstrainedQuadraticModel.add_discrete_from_comparison, and ConstrainedQuadraticModel.add_discrete_from_iterable.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (e7bc4d5) 94.86% compared to head (e55df2f) 94.86%.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1365   +/-   ##
=======================================
  Coverage   94.86%   94.86%           
=======================================
  Files          96       96           
  Lines       10324    10324           
=======================================
  Hits         9794     9794           
  Misses        530      530           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[arcondello]

This does need a release note. Probably something like

Add <kwarg name> keyword argument to `ConstrainedQuadraticModel.add_discrete()`, see `#1364 <https://github.com/dwavesystems/dimod/issues/1364>`_.

Also, ideally we would add tests to TestAddDiscrete

def test_overlap_check(self):
    cqm.add_discrete("abc")
    i = cqm.add_variable("INTEGER", "i")

    with self.assertRaises(ValueError):
        cqm.add_discrete("bcd", check_overlaps=True)

    cqm.add_discrete("bcd", check_overlap=False)  # allows overlapping

    with self.assertRaises(ValueError):
        cqm.add_discrete(i, check_overlap=False)  # still checks vartype

[arcondello]

I think check_valid or check_overlaps would be a better name, if we want to keep the "direction" of having it default to True.

[aivarsoo]

check_overlaps is a great name! I'll stick with that

[arcondello]

Suggested change

	the variables already exist, we make sure they're not used.
	the variables already exist, we make sure they're not used in another discrete constraint.

here and elsewhere.

[arcondello]

Suggested change

	if do_var_check and v in self.variables:
	# it already exists, let's make sure it's not already used
	if any(v in self.constraints[label].lhs.variables for label in self.discrete):
	if v in self.variables:
	# it already exists, let's make sure it's not already used
	if do_var_check and any(v in self.constraints[label].lhs.variables for label in self.discrete):

we still want to check that existing variables are binary, we just want to skip the check for overlapping constraints.

[arcondello]

FWIW, I think this keyword arg is sufficiently advanced that we don't need to add a docstring example for it.