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

[Mark Brown]

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/

> 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/

> 	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.

> 
> 	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/

> 	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.

> 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.

> +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.

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

> 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/

> @@ -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/

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