core.logic

to-stream fails on constraints

Details

  • Type: Defect Defect
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Completed
  • Affects Version/s: None
  • Fix Version/s: None
  • Component/s: None
  • Labels:
    None

Description

an example:
(defrel foo n)
(fact foo 1)
(fact foo 2)

(run* [q] (fresh [x] (!= x 1) (foo x)))
=> ((_0 :- (Unable to render embedded object: File (- () not found.= (2 1))))

The problem is that to-stream uses unify, which doesn't consider constraints. Here's two alternate implementations of stream (not replacements for to-stream, but basically performing the same task). One uses unify and the other uses bind and == to unify. The bind version produces the correct results.

(defn my-stream-unify [s v vals]
(when (seq vals)
(mplus (unify s v (first vals))
(λ [] (my-stream-unify s v (rest vals))))))

(defn my-stream-bind [s v vals]
(when (seq vals)
(mplus (bind s (== v (first vals)))
(λ [] (my-stream-bind s v (rest vals))))))

(defn myval-unify [x]
(fn [s]
(my-stream-unify s x [10 20 30])))

(defn myval-bind [x]
(fn [s]
(my-stream-bind s x [10 20 30])))

(run* [q]
(fresh [x]
(!= x 80)
(myval-unify x)))

(run* [q]
(fresh [x]
(!= x 80)
(myval-bind x)))

I've fixed my use of to-stream in pldb 0.1.3 in https://github.com/threatgrid/pldb/commit/8e8036fa0fedca7bb6ccee4af41a2bf6a66ffc2b

(db-rel foo n)
(with-db (db [foo 1]
[foo 2])
(doall (run* [q]
(fresh [x]
(!= x 1)
(foo x)))))

=> (_0)

If I've analyzed and fixed the problem correctly, I'd be happy to fix this small in core.logic. (and maybe it's time to get cracking on moving pldb into core.logic?)

Activity

Hide
Norman Richards added a comment -

Upon further reflection, to-stream really wasn't in error. It was the use of unify instead of ==. I didn't know how to apply the goal without mplus/bind. But, obviously, I can call the == goal directly to accomplish what bind was doing, and the existing choice in to-stream is sufficient to go what mplus was doing.

This second patch works on with the larger dbs I have and also passes the test case I added.

Show
Norman Richards added a comment - Upon further reflection, to-stream really wasn't in error. It was the use of unify instead of ==. I didn't know how to apply the goal without mplus/bind. But, obviously, I can call the == goal directly to accomplish what bind was doing, and the existing choice in to-stream is sufficient to go what mplus was doing. This second patch works on with the larger dbs I have and also passes the test case I added.
Hide
Norman Richards added a comment -

second version - doesn't blow the stack

Show
Norman Richards added a comment - second version - doesn't blow the stack
Hide
Norman Richards added a comment -

Did some more testing with this. It seems that the use of mplus here eats the stack up for larger data sets. I'm working on an alternate implementation that won't blow the stack.

Show
Norman Richards added a comment - Did some more testing with this. It seems that the use of mplus here eats the stack up for larger data sets. I'm working on an alternate implementation that won't blow the stack.
Hide
Norman Richards added a comment -

The problem is that we either need to fix that code or leave in the broken method. I guess the best approach is to move to-stream to datomic.clj, so I did that with the attached patch.

Show
Norman Richards added a comment - The problem is that we either need to fix that code or leave in the broken method. I guess the best approach is to move to-stream to datomic.clj, so I did that with the attached patch.
Hide
David Nolen added a comment -

Can we actually get the patch attached to the ticket as a file upload? Thanks!

As far as fixing Datomic support it's not high priority, it's an experiment not a feature

Show
David Nolen added a comment - Can we actually get the patch attached to the ticket as a file upload? Thanks! As far as fixing Datomic support it's not high priority, it's an experiment not a feature
Hide
Norman Richards added a comment -

https://github.com/orb/core.logic/commit/dd29620e792b55e6a114519f164de16c602a0d03.patch

Note - I could not test the change to the datomic use of to-stream, and there are no test cases covering it. If you don't have a way to verify that change (or would rather not touch that code) then we could not apply that part of the patch and leave the broken to-stream in for the datomic support.

Show
Norman Richards added a comment - https://github.com/orb/core.logic/commit/dd29620e792b55e6a114519f164de16c602a0d03.patch Note - I could not test the change to the datomic use of to-stream, and there are no test cases covering it. If you don't have a way to verify that change (or would rather not touch that code) then we could not apply that part of the patch and leave the broken to-stream in for the datomic support.
Hide
David Nolen added a comment -

The bind solution looks good to me. Please attach a patch, thanks!

And +1 to getting pldb into core.logic

Show
David Nolen added a comment - The bind solution looks good to me. Please attach a patch, thanks! And +1 to getting pldb into core.logic

People

Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: