[m-rev.] for review: consult knowledge base when adding suspects to the search space

[Ian MacLarty]

On Sat, 7 May 2005, Mark Brown wrote:

> Aside from the minor changes mentioned below, this looks fine.
>
> Cheers,
> Mark.
>
> On 05-May-2005, Ian MacLarty <maclarty at cs.mu.OZ.AU> wrote:
> > For review by anyone.
> >
> > Estimated hours taken: 10
> > Branches: main
> >
> > Look up atoms in the knowledge base as soon as the are added to the search
>
> s/the/they/
>

Fixed.

> > space.
> >
> > This generalises a feature of the debugger which I earlier removed
> > for simplicity, where the analyser would pass a list of questions to the
> > oracle.  If the oracle had any of the questions in its knowledge base it
> > would return those answers to the analyser, otherwise it would ask the user.
> > This was changed so that only one question was asked of the oracle at a time.
> > Now the analyser still asks only one question of the oracle at a time (through
> > an analyser response), but the oracle's knowledge base is consulted everytime a
>
> s/everytime/every time/

Fixed.

>
> > 	Move the code that checks if a bug has been found from
> > 	decide_analyser_response to the top-down search routine.  This is okay
> > 	since all the other search algorithms will eventually call top-down
> > 	search if they can't find any unknown suspects to ask questions about.
>
> It would be a good idea to document this point in the code.

Okay.  I've put the following comment in top_down_search where it checks if a bug
has been found :
			% Note that only top down search actually checks if a bug was
			% found.  This is okay, since all the other search algorithms
			% call top down search if they can't find an unknown suspect.

>
> >
> > 	When keeping track of the last unknown suspect, double check that the
> > 	suspect is still unknown before asking a question about it, since
> > 	it's status may have been changed because, for example, an erroneous
>
> s/it's/its/

Fixed.

>
> > 	Remove the suspect_is_bug predicate, since this is now done by top-down
> > 	search in the anaylser.  Export non_ignored_descendents so the
> > 	analyser can use it.
>
> Likewise with suspect_inadmissible.
>

Have added this comment to the CVS log.

> > Index: browser/declarative_analyser.m
> > ===================================================================
> > RCS file: /home/mercury1/repository/mercury/browser/declarative_analyser.m,v
> > retrieving revision 1.24
> > diff -u -r1.24 declarative_analyser.m
> > --- browser/declarative_analyser.m	2 May 2005 04:21:13 -0000	1.24
> > +++ browser/declarative_analyser.m	5 May 2005 05:25:00 -0000
>
> > -search(Store, !SearchSpace, follow_subterm_end(SuspectId, ArgPos, TermPath,
> > -		LastUnknown), FallBackSearchMode, NewMode, Response) :-
> > -	follow_subterm_end_search(Store, !SearchSpace, LastUnknown, SuspectId,
> > -		ArgPos, TermPath, FallBackSearchMode, NewMode, Response).
> > +search(IoActionMap, Store, Oracle, !SearchSpace, follow_subterm_end(SuspectId,
> > +		ArgPos, TermPath, LastUnknown), FallBackSearchMode,
> > +		NewMode, Response) :-
>
> This would be a bit more readable if you move the unification with
> follow_subterm_end/4 out of the head and into the body.
>

Done.

> > +find_middle_weight(IoActionMap, Store, Oracle, [], TopId, MaybeLastUnknown,
> > +		!SearchSpace, Response) :-
> >  	(
> >  		MaybeLastUnknown = yes(LastUnknown),
> > +		% Check that the last unknown suspect is
> > +		% still unknown (it might not be unknown if,
> > +		% for example, an erroneous suspect was added
> > +		% to the search space).
> > +		suspect_unknown(!.SearchSpace, LastUnknown)
>
> This same fragment of code appears in a number of places.  Even though it's
> only small, it would be worth making a separate semidet predicate out of it.
> That way the explanation given there in the comment would only need to be
> made once.

I have added a predicate suspect_still_unknown:

	% Check that a suspect is still unknown.  This is called by the search
	% algorithms to make double sure that a suspect is still unknown (it
	% might not be unknown if, for example, an erroneous suspect was added
	% to the search space during the search).
	%
:- pred suspect_still_unknown(search_space(T)::in, suspect_id::in) is semidet.

suspect_still_unknown(SearchSpace, SuspectId) :-
	suspect_unknown(SearchSpace, SuspectId).

>
> > Index: browser/declarative_debugger.m
> > ===================================================================
> > RCS file: /home/mercury1/repository/mercury/browser/declarative_debugger.m,v
> > retrieving revision 1.56
> > diff -u -r1.56 declarative_debugger.m
> > --- browser/declarative_debugger.m    2 May 2005 04:21:13 -0000       1.56
> > +++ browser/declarative_debugger.m    3 May 2005 03:19:10 -0000
> > @@ -194,6 +194,14 @@
> >                       % the last question asked.
> >       ;       show_info(io.output_stream).
> >
> > +     % Answers that are not necessarily obtained from the user.  These
> > +     % could be answers stored in the knowledge base or answers about
> > +     % trusted predicates.
> > +     %
> > +:- inst non_user_derived_answer
> > +     --->    truth_value(ground, ground)
> > +     ;       ignore(ground).
> > +
>
> I'd word that comment as "Answers that may be inferred from the knowledge
> base without consulting the user", or something like that, to make it
> clearer what role these play.  You could also mention answer_known/3 here
> as an example.
>
> Come to think of it, "non_user_derived_answer" is probably not the best
> name, since the answers were originally derived from some user at some
> stage.  Maybe "previously_known_answer" would be better?
>

I've change it to:

	% Answers that are known by the oracle without having to consult the user,
	% such as answers stored in the knowledge base or answers about trusted
	% predicates.  mdb.declarative_oracle.answer_known/3 returns answers of
	% this subtype.
	%
:- inst known_answer
	--->	truth_value(ground, ground)
	;	ignore(ground).

> > Index: browser/declarative_edt.m
> > ===================================================================
> > RCS file: /home/mercury1/repository/mercury/browser/declarative_edt.m,v
> > retrieving revision 1.7
> > diff -u -r1.7 declarative_edt.m
> > --- browser/declarative_edt.m	15 Apr 2005 05:42:34 -0000	1.7
> > +++ browser/declarative_edt.m	5 May 2005 06:16:10 -0000
> > @@ -261,25 +262,26 @@
> >  	%
> >  :- pred topmost_det(search_space(T)::in, suspect_id::out) is det.
> >
> > -	% suspect_is_bug(Store, SuspectId, !SearchSpace, CorrectDescendents,
> > -	%	InadmissibleChildren)
> > -	% Succeeds if the given suspect is erroneous and has only correct,
> > -	% inadmissible, pruned or ignored descendents.  The direct children of
> > -	% the root who are inadmissible are placed in InadmissibleChildren.
> > -	% CorrectDescendents is all the correct and inadmissible
> > -	% descendents of the suspect.
> > +	% non_ignored_descendents(IoActionMap, Store, Oracle, SuspectIds,
> > +	% 	!SearchSpace, Descendents).
> > +	% Descendents is the non-ignored children of the suspects in
> > +	% SuspectIds appended together.  If a child is ignored then it's
>
> s/it's/its/
>

Fixed.

> > @@ -334,8 +337,9 @@
> >
> >  			% The subterm originated from the suspect referenced by
> >  			% argument 1.  The second and third arguments give the
> > -			% position of the subterm in the origin node.
> > -	;	origin(suspect_id, arg_pos, term_path)
> > +			% position of the subterm in the origin node, while the
> > +			% 4th argument gives the mode of the subterm.
>
> s/4th/fourth/
>

Changed.

Cheers for that.

Ian.

--------------------------------------------------------------------------
mercury-reviews mailing list
post:  mercury-reviews at cs.mu.oz.au
administrative address: owner-mercury-reviews at cs.mu.oz.au
unsubscribe: Address: mercury-reviews-request at cs.mu.oz.au Message: unsubscribe
subscribe:   Address: mercury-reviews-request at cs.mu.oz.au Message: subscribe
--------------------------------------------------------------------------