<< Back to previous view

[LOGIC-139] to-stream fails on constraints Created: 23/Jun/13  Updated: 28/Jul/13  Resolved: 03/Jul/13

Status: Closed
Project: core.logic
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Norman Richards Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File 0001-fix-unification-on-relations.patch    

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



 Comments   
Comment by David Nolen [ 23/Jun/13 12:19 PM ]

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

And +1 to getting pldb into core.logic

Comment by Norman Richards [ 24/Jun/13 11:55 AM ]

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.

Comment by David Nolen [ 24/Jun/13 11:59 AM ]

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

Comment by Norman Richards [ 24/Jun/13 12:48 PM ]

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.

Comment by Norman Richards [ 25/Jun/13 5:33 PM ]

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.

Comment by Norman Richards [ 25/Jun/13 7:41 PM ]

second version - doesn't blow the stack

Comment by Norman Richards [ 25/Jun/13 7:46 PM ]

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.

Comment by David Nolen [ 03/Jul/13 8:28 PM ]

fixed http://github.com/clojure/core.logic/commit/d00d58685764a68d4e7c6d8294ac200786c83a7e

Generated at Fri Aug 22 10:44:16 CDT 2014 using JIRA 4.4#649-r158309.