clpBNR: bug in boolean constraints

[swi]

Putting this in a new thread to make it easier to find, but please see below for a new bug in the ‘implies’ (->) operator.

Bug in ~ operator (from other thread)

@swi

19 ?- { A==21r10, B==21r10, ~(A == B) }.
A = B, B = 21r10.

19 ?- { A==21r10, ~(A == A) }.
A = 21r10.

20 ?- { A==21r10, ~(A == A) }, solve(A).
A = 21r10.

I would expect false in all of them, is this a bug or am I missing something?

@ridgeworks

Definitely a bug, but easy to fix. I’m not in a position to push a new release just yet but I can provide instructions for the fix if you’re prepared to edit your copy of clpBNR.

Using boolean constraints this way is somewhat uncharted territory, so I hope nothing else turns up.

Bug in → operator?

25 ?- { (A == 1) -> (B == 2, C == 3) }.
false. % I would not expect this

26 ?- { (A == 1) -> (B == 2), (A == 1) -> (C == 3) }.
false. % I would not expect this

27 ?- { (A == 1) -> (B == 2) }, { (A == 1) -> (C == 3) } .
A::real(-1.0Inf, 1.0Inf), % Works fine
B::real(-1.0Inf, 1.0Inf),
C::real(-1.0Inf, 1.0Inf).

29 ?- { (A == 1) -> (B == 2) }, { (A == 1) -> (C == 3) }, A=1, solve([B,C]).
A = 1,  % Good, this is just testing the above with solve(...)
B = 2,
C = 3.

Am I missing something or is this a bug?

[swi]

@ridgeworks

I can provide instructions for the fix if you’re prepared to edit your copy of clpBNR.

sure, if you make a new github branch with the patches I can use that or I can make changes locally.

[ridgeworks]

@swi

Am I missing something or is this a bug?

I wouldn’t consider these bugs, but there is room for improvement. The first example (25) fails because currently “,” is only treated as a separator in top level constraint lists. In this particular case (embedded expression) you have to use and instead:

?- { (A == 1) -> (B == 2) and (C == 3) }.
A::real(-1.0Inf, 1.0Inf),
B::real(-1.0Inf, 1.0Inf),
C::real(-1.0Inf, 1.0Inf).

A reasonable enhancement would be to support “,” as a synonym for “and” in nested expressions.

The second (26) fails because it’s not recognized as a valid constraint due to the relative operator precedence of “->” and “,”. Instead:

?- { ((A == 1) -> (B == 2)), ((A == 1) -> (C == 3)) }.
A::real(-1.0Inf, 1.0Inf),
B::real(-1.0Inf, 1.0Inf),
C::real(-1.0Inf, 1.0Inf).

Both these cases will generate a debug message if clpBNR debugging is enabled.

[swi]

thanks, I suspected something about prcedence.

@ridgeworks

A reasonable enhancement would be to support “ , ” as a synonym for “ and ” in nested expressions.

I think this would be good for usability.

[ridgeworks]

@swi

sure, if you make a new github branch with the patches I can use that or I can make changes locally.

If you’re happy dealing with github, I’ve created a draft branch of the clpBNR repo (v0.9.8+fixes) with the fix for the neg primitive.

[ridgeworks]

@swi

I think this would be good for usability.

I tend to agree although “and” and “,” have different operator priorities and associativity. But it seems safe enough so I’ll try it for a while to see if anything trips up.

[swi]

@ridgeworks

I’ve created a draft branch of the clpBNR repo ( v0.9.8+fixes ) with the fix for the neg primitive.

Thanks,I tested it and the bug is solved.