search

srikumarks / FD.js

Finite domain constraint programming library in JS
62 stars 6 forks source link

!lt is not same as gt #6

Closed pvdz closed 8 years ago

pvdz commented 8 years ago

I hope I'm not overlooking something, but gt is defined as lt with reverse args. I think gt should call lte. Similar for gte calling lte.

It's weird because you define it correctly here: https://github.com/srikumarks/FD.js/blob/master/fd.js#L318

But then you define Space#lt by calling gt with reverse args. 

    Space.prototype.gt = function (v1name, v2name) {
        return this.lt(v2name, v1name);
    };

Same for gte.

pvdz commented 8 years ago

@srikumarks Hey. Um. I think I was wrong. I'm revisiting this code on my end and am realizing I'm confusing two things here.

There is the swapping of operand order to reduce code, like this PR is touching on. This is where x > y <=> y < x. Then there's inverting the operation, like when constructing reifiers: x > y <=> !(x <= y).

Case 1 is https://github.com/srikumarks/FD.js/commit/348756fe093ba24854ccb9a34cf39c3d556228cc (this PR). Case 2 is https://github.com/srikumarks/FD.js/blame/master/fd.js#L316

In short, I think this PR should be reverted. Suggest to add tests for this. It's a test case on my end that had me scratching my ears, realizing this mistake.

I'm very very sorry and feel a bit stupid.

pvdz commented 8 years ago

I've filed #7 for this.