LogtalkDotOrg / logtalk3

Logtalk - declarative object-oriented logic programming language
https://logtalk.org
Apache License 2.0
415 stars 30 forks source link

Strange behavior when extending arbitrary::arbitrary/2 predicate #121

Closed ccr185 closed 2 years ago

ccr185 commented 2 years ago

Hello!

I am observing some strange behavior when creating my own generators for subsequent use with quickcheck and the lgtunit which I suspect is somehow related to the interaction of the arbitrary library with the random library.

Here is the necessary code to reproduce the issue:

:- object(wwtypes).

:- multifile(arbitrary::arbitrary/1).
arbitrary::arbitrary(fluent).
:- multifile(arbitrary::arbitrary/2).
arbitrary::arbitrary(fluent, Arbitrary) :- 
   random::member(Functor, [at, out, carries, alive, dead]),
   handle_functor(Functor, Arbitrary).

:- private(handle_functor/2).
handle_functor(at, at(Entity, Place)) :-
   random::member(EntityBase, [agent, wumpus]),
   random::member(PlaceBase, [exit, wwplace, pit]),
   type::arbitrary(list(positive_integer,3),[X,Y,Id]),
   Entity=..[EntityBase, Id],
   Place=..[PlaceBase, X, Y].

:- end_object.

When testing this code, I observe the following very strange behaviour:

?- type::arbitrary(fluent, D).
D = at(agent(724), pit(93, 444)).

?- type::arbitrary(fluent, D).
false.

What's more, I am unable to step through the code with the debugger, i.e., running debugger::trace before performing the above calls has no effect, even when compiling with the debug(on) flag.

I do not see why this would be. I would greatly appreciate any help or ideas on how to minimize this issue.

I am running Logtalk 3.51.0 on ubuntu 20.04 LTS, installed from the .deb on the logtalk website, if it helps.

Thank you very much!

-ccr185

pmoura commented 2 years ago

Hi. In your code, the handle_functor/2 predicate have a fixed value, at, in the first argument. Therefore, when the random::member(Functor, [at, out, carries, alive, dead]) goal unifies Functor with one of the other atoms, the call to the handle_functor/2 predicate fails. For example:

?- {debugger(loader)}.
% [ /Users/pmoura/logtalk/tools/debugger/debuggerp.lgt loaded ]
% [ /Users/pmoura/logtalk/tools/debugger/debugger.lgt loaded ]
% [ /Users/pmoura/logtalk/tools/debugger/debugger_messages.lgt loaded ]
% [ /Users/pmoura/logtalk/tools/debugger/dump_trace.lgt loaded ]
% [ /Users/pmoura/logtalk/tools/debugger/loader.lgt loaded ]
% (0 warnings)
true.

?- debugger::trace.
   Debugger switched on: tracing everything for all objects compiled in debug mode.
true.

?- type::arbitrary(fluent, D).
   Rule: (0) arbitrary::arbitrary(fluent,_27512) ? 
   Call: (1) random::member(_28328,[at,out,carries,alive,dead]) ? 
   Exit: (1) random::member(at,[at,out,carries,alive,dead]) ? 
   Call: (2) handle_functor(at,_27512) ? 
   Rule: (2) handle_functor(at,at(_30614,_30616)) ? 
   Call: (3) random::member(_31138,[agent,wumpus]) ? 
   Exit: (3) random::member(agent,[agent,wumpus]) ? 
   Call: (4) random::member(_240,[exit,wwplace,pit]) ? 
   Exit: (4) random::member(pit,[exit,wwplace,pit]) ? 
   Call: (5) type::arbitrary(list(positive_integer,3),[_1512,_1518,_1524]) ? 
   Exit: (5) type::arbitrary(list(positive_integer,3),[93,444,724]) ? 
   Call: (6) _226=..[agent,724] ? 
   Exit: (6) agent(724)=..[agent,724] ? 
   Call: (7) _228=..[pit,93,444] ? 
   Exit: (7) pit(93,444)=..[pit,93,444] ? 
   Exit: (2) handle_functor(at,at(agent(724),pit(93,444))) ? 
D = at(agent(724), pit(93, 444)).

?- type::arbitrary(fluent, D).
   Rule: (7) arbitrary::arbitrary(fluent,_7410) ? 
   Call: (8) random::member(_8108,[at,out,carries,alive,dead]) ? 
   Exit: (8) random::member(dead,[at,out,carries,alive,dead]) ? 
   Call: (9) handle_functor(dead,_7410) ? 
   Fail: (9) handle_functor(dead,_7410) ? 
false.

A possible fix would be to rewrite the handle_functor/2 predicate as:

:- private(handle_functor/2).
handle_functor(Functor, Term) :-
   Term =.. [Functor, Entity, Place],
   random::member(EntityBase, [agent, wumpus]),
   random::member(PlaceBase, [exit, wwplace, pit]),
   type::arbitrary(list(positive_integer,3),[X,Y,Id]),
   Entity=..[EntityBase, Id],
   Place=..[PlaceBase, X, Y].

This will give:

?- {*}.
% Redefining object wwtypes
% [ /Users/pmoura/arb/arb.lgt reloaded ]
% (0 warnings)
% Reloaded all Logtalk source files modified or that required
% recompilation due to a change to the compilation mode
true.

?- debugger::nodebug.
   Debugger switched off.
true.

?- type::arbitrary(fluent, D).
D = carries(agent(312), wwplace(946, 502)).

?- type::arbitrary(fluent, D).
D = dead(wumpus(667), wwplace(598, 916)).

?- type::arbitrary(fluent, D).
D = carries(agent(143), exit(478, 597)).

?- type::arbitrary(fluent, D).
D = alive(agent(160), wwplace(210, 698)).

?- type::arbitrary(fluent, D).
D = out(agent(458), wwplace(559, 215)).
ccr185 commented 2 years ago

I don't know how I didn't see that, thank you so much for the prompt response

pmoura commented 2 years ago

P.S. When defining clauses for multifile predicates that only call local, auxiliary predicates, as in your case here, best to encapsulate them in a category instead of in an object as there's really no public interface for the object.

ccr185 commented 2 years ago

P.S. When defining clauses for multifile predicates that only call local, auxiliary predicates, as in your case here, best to encapsulate them in a category instead of in an object as there's really no public interface for the object.

I see, note taken :)

Thank you very much both for your help and your promptness in responding!

ccr185 commented 2 years ago

Hi!

I am so sorry to take so much of your time, but I have followed your advice and now my generators work perfectly, however, I am seeing quickcheck behave very strangely, and it does not seem related to the aforementioned problem.

To illustrate, here's what's happening concretely:

?- lgtunit::quick_check(tests::holds_prop(+non_empty_list(fluent)),[v(true)]).
*     Failure:   tests::holds_prop([at(agent(946),pit(444,724)),dead(wumpus(502)),out(wumpus(312)),dead(wumpus(598))])
*     quick check test failure (at test 1 after 0 shrinks):
*       tests::holds_prop([at(agent(946),pit(444,724)),dead(wumpus(502)),out(wumpus(312)),dead(wumpus(598))])
*     starting seed: seed(3172,9814,20125)
false.

?- tests::holds_prop([at(agent(946),pit(444,724)),dead(wumpus(502)),out(wumpus(312)),dead(wumpus(598))]).
true.

That is, it seems the goal fails unexpectedly even when called normally it does succeed, which would run counter to what one would expect. When I run through it with the debugger, it seems to exit the goal just fine, but for some reason decides to backtrack and ultimately fails. Are you able to reproduce this issue on your end?

If it helps, here's the definition of the property in question and an alternative definition of the QuickCheck test as part of a test suite that exhibits the same behavior:

:- public(holds_prop/1).
    holds_prop(Fluents) :-
        forall(member(Fluent, Fluents),::holds(Fluent, Fluents)).

    quick_check(
        holds,
        (holds_prop(+non_empty_list(fluent))),
        [v(true),n(10)]
    ).

In addition, here's the definition of the predicate I'm testing:

holds(F, [F|_]).
holds(F, Z) :- Z=[F1|Z1], F\==F1, holds(F, Z1).

(I know the property itself is rather trivial, but this is more to test the approach than anything else).

Thank you again in advance,

-ccr

P.S. Perhaps it's worth mentioning that I'm using SWI as my back-end. P.S. N.2 Please let me know if I should refile this under a separate issue instead.

pmoura commented 2 years ago

Where is the predicate holds/2 defined? In the same object that contains the holds_prop/1 predicate? If so, why the message to self in the forall/2 call?

ccr185 commented 2 years ago

It is defined in a category outside the test suite, which imports it (I though perhaps the easiest way to get it in there since it's a category I use in other objects)

ccr185 commented 2 years ago

Here's a more complete version of the test object:

:- object(tests, extends(lgtunit), imports(sflux)).
    :- public(holds_prop/1).
    holds_prop(Fluents) :-
        forall(member(Fluent, Fluents),::holds(Fluent, Fluents)).

    quick_check(
        holds,
        (holds_prop(+non_empty_list(fluent,5))),
        [v(true),n(10)]
    ).
:- end_object.

And then you would have:


:- category(sflux).
:- public(holds/2).
holds(F, [F|_]).
holds(F, Z) :- Z=[F1|Z1], F\==F1, holds(F, Z1).
:- end_category.
pmoura commented 2 years ago

Does the tests object imports that category?

ccr185 commented 2 years ago

it does indeed, would that be an issue? Would that be a bad practice?

ccr185 commented 2 years ago

Where is the predicate holds/2 defined? In the same object that contains the holds_prop/1 predicate? If so, why the message to self in the forall/2 call?

The message to self, I took it from the documentation that says to call imported predicates from categories in this manner.

ccr185 commented 2 years ago

Even changing the property to the following still produces the same behavior (i.e. calling an object that itself imports the sflux category):

holds_prop(Fluents) :-
        forall(member(Fluent, Fluents),wwstate(_,_)::holds(Fluent, Fluents)).
pmoura commented 2 years ago

There are two bugs in your code:

  1. There's no non_empty_list(Type, Length) type. That causes an immediate failure (for performance reasons, the existence of the type is not checked).
  2. There's a definition for generating arbitrary values for the type fluent but no definition that verifies that a term is a valid fluent.

The first bug is easy to correct. Use one of the other defined list types (see https://logtalk.org/library/arbitrary_0.html).

The second bug requires defining clauses for the multifile predicates type::type/1 and type::check/2 predicates (see https://logtalk.org/library/type_0.html) so that terms of type fluent can be checked (which is required by the QuickCheck implementation).

ccr185 commented 2 years ago

Thank you so much, you're absolutely right. What an embarrassing mistake, it's not an issue at all. I'd suggest (and perhaps humbly ask) that you delete this issue altogether, if possible.

pmoura commented 2 years ago

Nothing to be embarrassed about. Happy logtalking!