dwavesystems / dimod

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

Initializing BinaryQuadraticModel using keyword arguments gives empty model #760

Open mstechly opened 3 years ago

mstechly commented 3 years ago

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

Additional Context CC: @dexter2206

arcondello commented 3 years ago

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?

mstechly commented 3 years ago

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 :)