dimod icon indicating copy to clipboard operation
dimod copied to clipboard

Initializing BinaryQuadraticModel using keyword arguments gives empty model

Open mstechly opened this issue 4 years ago • 2 comments

Description When we have tried initializing BinaryQuadraticModel using all the keyword arguments, we got an empty model. However, if we use linear, quadratic and offset as positional arguments, it works properly. We understand that they might be purely positional, but the initializer should at least raise an exception when unrecognized arguments are passed.

Steps To Reproduce This gives expected behaviour:

$ dimod.BinaryQuadraticModel({1:0.5, 2:0.5 }, {(1,2): 0.5}, 2, vartype="SPIN")
BinaryQuadraticModel({1: 0.5, 2: 0.5}, {(1, 2): 0.5}, 2.0, 'SPIN')

This is unexpected behaviour:

$ dimod.BinaryQuadraticModel(linear={1:0.5, 2:0.5 }, quadratic={(1,2): 0.5}, offset=2, vartype="SPIN")
dimod.BinaryQuadraticModel({1:0.5, 2:0.5 }, {(1,2): 0.5}, offset=2, vartype="SPIN")

This is also unexpected behaviour:

$ dimod.BinaryQuadraticModel(linear={1:0.5, 2:0.5 }, quadratic={(1,2): 0.5}, offset=2, vartype="SPIN")
BinaryQuadraticModel({}, {}, 0.0, 'SPIN')

Specifying linear twice, once as positional and once as keyword argument is also confusing:

$ dimod.BinaryQuadraticModel({1:0.5, 2:0.5 }, {(1,2): 0.5}, 2.0, linear={1:0.7, 2:0.2}, vartype="SPIN")
BinaryQuadraticModel({1: 0.5, 2: 0.5}, {(1, 2): 0.5}, 2.0, 'SPIN')

Also, all the other keyword arguments are ignored without throwing a warning or exception.

Expected Behavior Passing linear, quadratic and offset as keyword arguments should work or raise an exception.

Environment

  • OS: MacOS 10.15.6
  • Python version: Python 3.7.9
  • dimod version: dimod==0.9.11

Additional Context CC: @dexter2206

mstechly avatar Nov 24 '20 16:11 mstechly

This is a known issue. The signature actually doesn't have any named arguments except vartype.

We could add named kwargs for convenience, though obviously it would complicate the implementation. Totally agree it should at least raise an exception.

Thanks for the bug report! We're doing some refactoring of the construction code soon anyway, unless this is blocking y'all now?

arcondello avatar Nov 24 '20 16:11 arcondello

I think that just throwing a warning/exception wouldn't complicate the implementation much and would be very helpful for the users and would protect them from making a mistake.

Also, the docs for the class are not mentioning that these arguments are purely positional.

It's not blocking, just reporting it for future generations. We're glad you're working on it :)

mstechly avatar Nov 24 '20 16:11 mstechly