scryer-prolog icon indicating copy to clipboard operation
scryer-prolog copied to clipboard

unexpected variables in the output

Open josd opened this issue 1 year ago • 36 comments

We get unexpected variables in the output of the following test

cd /tmp
git clone https://github.com/eyereasoner/eye2
cd eye2
git checkout tags/v1.0.6
scryer-prolog -g run eye3.pl etc/bmt.pl > etc/output/bmt.pl
git diff

So we see

'urn:example:cycle'('urn:example:i0',['urn:example:i12','urn:example:i66','urn:example:i9','urn:example:i12','urn:example:i66','urn:example:i9','urn:example:i12','urn:example:i9','urn:example:i12','urn:example:i9',_1088720]) => true.

instead of

'urn:example:cycle'('urn:example:i0',['urn:example:i12','urn:example:i66','urn:example:i9','urn:example:i12','urn:example:i66','urn:example:i9','urn:example:i12','urn:example:i9','urn:example:i12','urn:example:i9','urn:example:i12']) => true.

that is compared with the output of

tpl -g run eye3.pl etc/bmt.pl > etc/output/bmt.pl

I was not able to make a simplified case to show the issue.

josd avatar Nov 24 '24 19:11 josd

This is tested on

si
OS: Ubuntu 22.04.5 LTS, CPU: AMD Ryzen 5 7520U with Radeon Graphics, RAM: 3705676 kB

and with

scryer-prolog --version
v0.9.4-210-gff034326

josd avatar Nov 24 '24 20:11 josd

The test is now at https://github.com/eyereasoner/eyelet/blob/main/input/basic-monadic.pl The result can be reproduced as

cd /tmp
git clone https://github.com/eyereasoner/eyelet
cd eyelet
scryer-prolog -g main eyelet.pl input/basic-monadic.pl

It is still giving

answer(cycle(i0,[i12,i66,i9,i12,i66,i9,i12,i9,i12,i9,A])).

instead of

answer(cycle(i0,[i12,i66,i9,i12,i66,i9,i12,i9,i12,i9,i12])).

josd avatar Apr 30 '25 12:04 josd

This is tough: It's a real mistake in the engine, appearing in a test that uses the combination of quite a few different and quite elementary language constructs to elicit the issue, and it arises also with latest rebis-dev which we thought was already close to being ready.

One thing I found out is that if I pull the first or more elements out of the list and instead write them as additional arguments, then the example seems to work as expected. See the patch below for an example that apparently makes the test work.

@haijinSk: Since you have a great talent to reduce such examples to much more manageable fragments that still exhibit the issue, I would greatly appreciate if you could take a look at this issue and try to remove all unnecessary aspects, or to gain insights into which parts of the engine are most likely responsible. Maybe at least a few of the facts can be removed? I think you can use Trealla Prolog as a reference to test against?

A patch that seems to make the test work as expected follows. Thank you a lot!

diff --git a/input/basic-monadic.pl b/input/basic-monadic.pl
index b58a679..a4c42d7 100644
--- a/input/basic-monadic.pl
+++ b/input/basic-monadic.pl
@@ -10004,7 +10004,7 @@ i9(i9, i54).
 i9(i9, i84).
 i9(i9, i96).
 
-cycle(A, [B, C, D, E, F, G, H, I, J, K, B]) :-
+cycle(A, B, [C, D, E, F, G, H, I, J, K, B]) :-
     current_predicate(A/2),
     sub_atom(A, 0, 1, _, i),
     A \= cycle,
@@ -10020,5 +10020,5 @@ cycle(A, [B, C, D, E, F, G, H, I, J, K, B]) :-
     call(A, K, B).
 
 % query
-true :+ cycle(_, _).
-true :+ \+cycle(_, _).
+true :+ cycle(_, _, _).
+true :+ \+cycle(_, _, _).

triska avatar Apr 30 '25 21:04 triska

I found another way to write the example that seems to yield the expected result: It works if I avoid the aliased variable B which occurs twice in the clause head, by using the fresh variable X instead which I explicitly unify to B in the body of the clause:

diff --git a/input/basic-monadic.pl b/input/basic-monadic.pl
index b58a679..17e25d4 100644
--- a/input/basic-monadic.pl
+++ b/input/basic-monadic.pl
@@ -10004,11 +10004,12 @@ i9(i9, i54).
 i9(i9, i84).
 i9(i9, i96).
 
-cycle(A, [B, C, D, E, F, G, H, I, J, K, B]) :-
+cycle(A, [B, C, D, E, F, G, H, I, J, K, X]) :-
     current_predicate(A/2),
     sub_atom(A, 0, 1, _, i),
     A \= cycle,
     call(A, B, C),
+    X = B,
     call(A, C, D),
     call(A, D, E),
     call(A, E, F),

When I move the unification further up in the body, the issue again arises.

triska avatar Apr 30 '25 21:04 triska

Thanks @triska for digging into this and I feel that it will work out :-) This was a test we made up at https://eulersharp.sourceforge.net/2003/03swap/bmb-note to see how SAS technology https://www.sas.com/en_be/home.html would deal with it.

josd avatar Apr 30 '25 22:04 josd

The following program basic.pl suffices to exhibit the issue:

:- op(1200, xfx, :+).

i(a, b).

cycle([A, B, A]) :-
        i(A, B).

true :+ cycle(_).

Yielding:

$ scryer-prolog -g main eyelet.pl basic.pl
:- op(1200, xfx, :+).

answer(cycle([a,b,_20714])).

step((true:+cycle(A)),cycle([a,b,_20757]),true).

triska avatar Apr 30 '25 22:04 triska

@josd: Please see the small fragment above that still exhibits the issue. Does it help to trace it down further?

triska avatar Apr 30 '25 22:04 triska

Very nifty and indeed

scryer-prolog -g main eyelet.pl /tmp/basic.pl
:- op(1200, xfx, :+).

answer(cycle(A,[a,b,B])).

step((true:+cycle(A,B)),cycle(C,[a,b,D]),true).

josd avatar Apr 30 '25 22:04 josd

compared with

tpl -g main eyelet.pl /tmp/basic.pl
:- op(1200, xfx, :+).

answer(cycle(A,"aba")).

step((true:+cycle(A,B)),cycle(C,"aba"),true).

josd avatar Apr 30 '25 22:04 josd

I have shortened everything to the following interaction which I think shows the essence of the issue:

$ scryer-prolog
?- L = [A,A], A = a, assertz(l(L)).
   L = "aa", A = a.
?- l(L).
   L = [a,_A], unexpected.
   L = "aa". % expected

triska avatar Apr 30 '25 23:04 triska

Interestingly, only the first variable is going to be instantiated with l/1 query:

?- L = [A,A,A], A=a, asserta(l(L)).
    L = "aaa", A = a.
?- l(L).
   L = [a,_A,_A], unexpected.
   L = "aaa". % expected, but not found.

haijinSk avatar May 03 '25 11:05 haijinSk

But, as expected:


?- L = [A,B,C], L = [a,a,a], asserta(l(L)).
    L = "aaa", A = a, B = a, C = a.
?- l(L).
    L = "aaa". % as expected.
?- L = [A,B,C], A=a,B=a,C=a, assertz(l(L)).
   L = "aaa", A = a, B = a, C = a.
?- l(L).
   L = "aaa". % as expected.

haijinSk avatar May 03 '25 11:05 haijinSk

Even better (as expected):

?- [user].
test :-
  L = [A,A,A],
  A=a,
  assertz(l(L)).


?- test.
   true.
?- l(L).
   L = "aaa". % as expected.

haijinSk avatar May 03 '25 12:05 haijinSk

Still better (as expected):

?- L = [A,A,A], A=a,copy_term(L,L2), assertz(l(L2)).
   L = "aaa", A = a, L2 = "aaa".
?- l(L).
   L = "aaa". % as expected.

But (unexpected):

?- L = [A,A,A], A=a, L = L2, assertz(l(L2)).
   L = "aaa", A = a, L2 = "aaa".
?- l(L).
   L = [a,_A,_A], unexpected.
   L = "aaa". % expected, but not found.

haijinSk avatar May 03 '25 12:05 haijinSk

PS: In all my examples above I used rebis-dev: https://github.com/mthom/scryer-prolog/tree/52363140707b29a20b0f7461288cad480b8d50a7

haijinSk avatar May 03 '25 12:05 haijinSk

Good work and I wonder whether it could have to do with

$ scryer-prolog
?- L = [A,A,A], A=a, asserta(l(L)).
   L = "aaa", A = a.
?- l([A,B,C]).
   A = a, B = C.
?-

whereas trealla says

$ tpl
?- L = [A,A,A], A=a, asserta(l(L)).
   L = "aaa", A = a.
?- l([A,B,C]).
   A = a, B = a, C = a.
?-

josd avatar May 03 '25 22:05 josd

As I understand/see it, yes; it expresses, in a different form, that (wrongly in that case) only the first element of the list/string was instantiated in the Scryer's "database".

As if (here as expected):

$ tpl
?- L = [A,B,B], A=a, asserta(l(L)).
   L = [a,B,B], A = a.
?- l([A,B,C]).
   A = a, C = B. % as expected.

haijinSk avatar May 04 '25 07:05 haijinSk

Indeed this shows something interesting:

?- l([A,B,C]).
   A = a, B = C.

namely that the last two elements (B and C) are correctly aliased, but their relation with A is missing.

triska avatar May 04 '25 08:05 triska

And further, the issue may be confined to lists. For comparison, the following works exactly as expected:

?- T = f(A,f(A,[])), A = a, assertz(l(T)).
   T = f(a,f(a,[])), A = a.
?- l(T).
   T = f(a,f(a,[])). % expected

triska avatar May 04 '25 08:05 triska

Yes!

?-  T = f([A,A,A]), A = a, assertz(l(T)).
   T = f("aaa"), A = a.
?- l(T).
   T = f("aaa"). % as expected.

haijinSk avatar May 04 '25 08:05 haijinSk

As I see it, it's as if Scryer is "forced more", in those/certain cases, to instantiate all variables (that should be) before putting the fact to its database, so to speak.

haijinSk avatar May 04 '25 08:05 haijinSk

For comparison, just using . instead of f:

?- T = f(A,f(A,[])), A = a, assertz(l(T)).
   T = f(a,f(a,[])), A = a.
?- T = .(A,.(A,[])), A = a, assertz(l(T)).
   T = "aa", A = a.

Up to here, the interaction is completely as expected.

But then we have:

?- l(Ls).
   Ls = f(a,f(a,[])) % expected
;  Ls = [a,_A], unexpected.

triska avatar May 04 '25 08:05 triska

And also, as already noted in https://github.com/mthom/scryer-prolog/issues/2668#issuecomment-2848598803, everything works exactly as expected when the exact same goals appear as part of a program instead of appearing only as part of a query on the toplevel:

$ scryer-prolog -f
?- [user].
t :- Ls = [A,A], A = a, assertz(l(Ls)).

?- t.
   true.
?- l(Ls).
   Ls = "aa". % expected

So, maybe a binding that should exist between variables in the clause database is unexpectedly undone on backtracking which arises implicitly between toplevel queries? It would be interesting to check whether other constructs besides assertz/1 are also affected by this or similar issues. All-solution predicates maybe? If you are interested in this, I would greatly appreciate an extensive check for similar cases in related constructs!

triska avatar May 04 '25 08:05 triska

In some my examples I used asserta/1, so my naive/obvious speculation is, that affected is "everything" that uses the same underlying mechanism, whatever it is, so to speak; maybe only the "assert things" or maybe it is worse, as this issue didn't started as a top-level issue.

haijinSk avatar May 04 '25 09:05 haijinSk

We have another case https://github.com/eyereasoner/eyelet/blob/main/input/enigma1225.pl with unexpected variables in the output and this time they are not the last element of a list:

~/github.com/eyereasoner/eyelet$ scryer-prolog -g main eyelet.pl input/enigma1225.pl
:- op(1200, xfx, :+).

answer(enigma1225(8,[[2,1,4,3,6,5,8,7],[[1,A,2,3,4,5,6,7],[1,1,3,2,5,4,7,6],[3,2,8,B,9,10,11,12],[2,3,8,8,10,9,12,11],[5,4,10,9,13,C,14,15],[4,5,9,10,13,13,15,14],[7,6,12,11,15,14,16,D],[6,7,11,12,14,15,16,16]],544])).

step((true:+enigma1225(8,A)),enigma1225(8,[[2,1,4,3,6,5,8,7],[[1,B,2,3,4,5,6,7],[1,1,3,2,5,4,7,6],[3,2,8,C,9,10,11,12],[2,3,8,8,10,9,12,11],[5,4,10,9,13,D,14,15],[4,5,9,10,13,13,15,14],[7,6,12,11,15,14,16,E],[6,7,11,12,14,15,16,16]],544]),true).

expected

~/github.com/eyereasoner/eyelet$ tpl -g main eyelet.pl input/enigma1225.pl
:- op(1200, xfx, :+).

answer(enigma1225(8,[[2,1,4,3,6,5,8,7],[[1,1,2,3,4,5,6,7],[1,1,3,2,5,4,7,6],[3,2,8,8,9,10,11,12],[2,3,8,8,10,9,12,11],[5,4,10,9,13,13,14,15],[4,5,9,10,13,13,15,14],[7,6,12,11,15,14,16,16],[6,7,11,12,14,15,16,16]],544])).

step((true:+enigma1225(8,A)),enigma1225(8,[[2,1,4,3,6,5,8,7],[[1,1,2,3,4,5,6,7],[1,1,3,2,5,4,7,6],[3,2,8,8,9,10,11,12],[2,3,8,8,10,9,12,11],[5,4,10,9,13,13,14,15],[4,5,9,10,13,13,15,14],[7,6,12,11,15,14,16,16],[6,7,11,12,14,15,16,16]],544]),true).

josd avatar May 04 '25 10:05 josd

How can I test it?

$ scryer-prolog -g main eyelet.pl basic-monadic.pl
*** error(instantiation_error,instantiation_error(unknown(A),1))

see https://github.com/mthom/scryer-prolog/issues/2668#issuecomment-2841827711

josd avatar May 04 '25 10:05 josd

Thank you, now I have it...

haijinSk avatar May 04 '25 10:05 haijinSk

What about adding copy_term/2 here (eyelet.pl), before assertz/1:

% assert conjunction
assert_conj((B, C)) :-
    assert_conj(B),
    assert_conj(C).
assert_conj(A) :-
    (   \+ A
    ->  copy_term(A,A1), assertz(A1) % !!!
    ;   true
    ).

haijinSk avatar May 04 '25 10:05 haijinSk

After the addition, now I get:

scryer-prolog -g main eyelet.pl enigma1225.pl 
:- op(1200, xfx, :+).

answer(enigma1225(8,[[2,1,4,3,6,5,8,7],[[1,1,2,3,4,5,6,7],[1,1,3,2,5,4,7,6],[3,2,8,8,9,10,11,12],[2,3,8,8,10,9,12,11],[5,4,10,9,13,13,14,15],[4,5,9,10,13,13,15,14],[7,6,12,11,15,14,16,16],[6,7,11,12,14,15,16,16]],544])).

step((true:+enigma1225(8,A)),enigma1225(8,[[2,1,4,3,6,5,8,7],[[1,1,2,3,4,5,6,7],[1,1,3,2,5,4,7,6],[3,2,8,8,9,10,11,12],[2,3,8,8,10,9,12,11],[5,4,10,9,13,13,14,15],[4,5,9,10,13,13,15,14],[7,6,12,11,15,14,16,16],[6,7,11,12,14,15,16,16]],544]),true).

haijinSk avatar May 04 '25 11:05 haijinSk

Wow, now basic-monadic and enigma1225 cases work fine! Thanks @haijinSk 👍 So we now have

$ scryer-prolog
?- L = [A,A], A = a, assertz(l(L)), l(M).
   L = "aa", A = a, M = [a,_A].    % unexpected
?-
$ scryer-prolog
?- L = [A,A], A = a, copy_term(L, L2), assertz(l(L2)), l(M).
   L = "aa", A = a, L2 = "aa", M = "aa".    % expected
?-

josd avatar May 04 '25 11:05 josd