[m-rev.] for review: using dice information in the declarative debugger

[Zoltan Somogyi]

On 05-Aug-2005, Ian MacLarty <maclarty at cs.mu.OZ.AU> wrote:
> For review by anyone.
> 
> Estimated hours taken: 30
> Branches: main
> 
> Use dicing information in the declarative debugger.  Each label in the 
> program
> is assigned a suspicion based on a supplied dice.  A new search mode then
> performs divide and query using the total suspicion of a subtree as the
> weighting of the subtree.

There is lots of funny indentation in the log message.

> browser/declarative_analyser.m:
> 	Parameterize the divide and query search mode by allowing it to work
> 	with an arbitrary weighting heuristic.
> 
> 	Support two weighting heuristics: number of events and suspicion.
> 
> 	Since there is still only one weight field for each suspect,
> 	if the weighting heuristic changes, then update all the weights of 
> 	all
> 	the suspects.
> 
> 	Return a different reason for asking a divide and query question,
> 	depending on the weighting heuristic.
> 
>  	Some information (specifically how many suspect events remain and
> 	the estimated number of questions remaining for divide and query)
> 	returned by the info command depends on the current weighting 
> 	heuristic
> 	being the number of events.  If the current weighting heuristic is 
> 	not
> 	the number of events then do not show this information.
> 
> browser/declarative_debugger.m:
> 	Pass the trace node store to set_fallback_search_mode so that the
> 	weights can be recalculated if the search strategy changes.
> 
> browser/declarative_edt.m:
> 	In the mercury_edt typeclass, rename the edt_weight method to
> 	edt_number_of_events and add a new method edt_subtree_suspicion.
> 
> 	The weight field of each suspect in the search space can either
> 	be based on suspicion or number of events.
> 	Add a field to the search_space type to determine which weighting
> 	heuristic to use.  Export predicates to get and set the current
> 	weighting heuristic being used.  If the weighting heuristic
> 	changes the recalculate the weights of all the suspects.
> 
> 	When calculating the weight of a suspect use the current weighting
> 	heuristic.
> 
> browser/declarative_execution.m:
> 	Record a suspicion accumulator at each interface event which
> 	can be used to calculate the suspicion of a subtree in the EDT.
> 
> 	Move the label_layout and proc_layout types as well as all utility
> 	predicates for those types to a new module, mdbcomp.label_layout.
> 
> browser/declarative_oracle.m:
> browser/declarative_user.m:
> browser/debugger_interface.m:
> 	Import mdbcomp.label_layout.
> 
> browser/declarative_tree.m:
> 	Adjust for the extra field in interface nodes in the annotated trace.
> 
> 	Look at the weighting heuristic when calculating the weight of a
> 	subtree.
> 
> browser/util.m:
> mdbcomp/program_representation.m:
> 	Move goal_path_string to mdbcomp.program_representation since
> 	it is needed in mdbcomp.label_layout.
> 
> doc/user_guide.texi:
> 	Document the new search mode.
> 
> mdbcomp/label_layout.m:
> 	This module contains the types label_layout and proc_layout and
> 	supporting predicates which were in mdb.declarative_execution.
> 	These types are needed in the mdbcomp.slice_and_dice module.

I think something like rtti_access would be a better name for this module:
it contains proc_layouts as well as label layouts, and maybe other kinds
of RTTI as well in the future.

> mdbcomp/mdbcomp.m:
> 	Include label_layout.
> 
> mdbcomp/slice_and_dice.m:
> 	Add functions for calculating different suspicion formulas.  The
> 	intention is to experiment with different formulas in the future.
> 
> 	Export predicates for reading a dice from the C backend.
> 
> 	Export a predicate for retrieving the suspicion of a label
> 	given a dice.  This predicate uses the suspicion_ratio_binary
> 	formula, since that seems most effective in my (as yet very limited)
> 	experience.  I will implement better ways to control and customise
> 	the formula used in the future.
> 
> mdbcomp/trace_counts.m:
> 	Add a function for constructing a path_port given a goal path and
> 	a trace port.
> 
> 	If there is an unexpected exception when reading a trace counts
> 	file then print the unexpected exception.
> 
> 	Add a predicate to convert trace count file types to a string and
> 	vica versa.
> 
> runtime/mercury_stack_layout.h:
> 	Fix a typo.
> 
> runtime/mercury_trace_base.c:
> runtime/mercury_trace_base.h:
> 	Export a function to look up the trace count slot for a label.
> 	Use this function when recording trace counts.
> 	This function will also be used in the declarative debugger backend 
> 	to
> 	look up suspicions for labels.
> 
> 	Add a function to initialise the array which records which ports
> 	need a goal path to uniquely identifiy the label.
> 	Initially I used this array elsewhere which is why I exported it.
> 	I didn't actually end up needing to use it in the final version,
> 	but I'm still exporting it, since it might be useful in the future.
> 
> tests/debugger/declarative/Mmakefile:
> tests/debugger/declarative/dice.exp:
> tests/debugger/declarative/dice.inp:
> tests/debugger/declarative/dice.m:
> 	Test the new search mode.
> 
> tests/debugger/declarative/info.exp:
> 	The weigting heuristic is now printed with the info command.
> 	Also some information, such as the number of suspect events,
> 	is no longer printed if the weigthing heuristic is not the number
> 	of events (since then that information is not available).
> 
> trace/mercury_trace_declarative.c:
> 	Add a function to setup the trace counts array with a suspicion for
> 	each label.  For efficiency the suspicion is converted from a
> 	float to an integer between 0 and 100.
> 
> 	If a flag is set, then increment an accumulator with the
> 	suspicion of each label executed as the annotated trace is being
> 	constructed.  Store the value of the accumulator at interface events,
> 	so that the frontend can efficiently calculate the suspicion of any
> 	subtree.
> 
> 	Remove a redundant variable and comment: the goal path is no
> 	longer passed to the frontend, because the frontend has access to
> 	the label_layout from which it can get the goal path (the variable 
> 	and
> 	comment are artifacts of a previous change).
> 
> 	When checking if a search mode is valid also check if failing and
> 	passing trace counts are required for the search mode.
> 	Allow abbreviations for the search mode arguments.
> 
> trace/mercury_trace_declarative.h:
> 	Export the predicate to set up the suspicions for each label.
> 
> trace/mercury_trace_internal.c:
> 	Allow passing and failing test case(s) to be passed
> 	as arguments to the dd command.
> 
> 	If passing and failing test case(s) are supplied then record
> 	suspicions in the annotated trace even if the sdq search mode
> 	is not specified.  The user could switch to the sdq search mode later
> 	on.
> 
> 	Initialise some values which were causing warnings from the C
> 	compiler.

>  	;	divide_and_query(
> -			old_weight		:: int,
> -			choosen_subtree_weight 	:: int
> +			dq_weighting			:: 
> weighting_heuristic,
> +				% The weight of the search space before the 
> question
> +				% was asked.
> +			dq_old_weight			:: int,
> +				% The weight the searchspace will be if the 
> user answers
> +				% `no' to the current question.
> +			dq_choosen_subtree_weight 	:: int

It is not clear which field each comment applies to. Also, check
the indentation here.

>  			reset_analyser(!Analyser),
> -			initialise_search_space(Store, Node, SearchSpace),
> +			MaybeWeighting = 
> get_maybe_weighting_from_search_mode(
> +				!.Analyser ^ search_mode),

And here.

> -search(Store, Oracle, !SearchSpace, divide_and_query, _, NewMode, 
> +search(Store, Oracle, !SearchSpace, divide_and_query(Weighting), _, 
> NewMode,
>  		Response) :-

And here, and in lots more places.

> +weighting_to_reason_string(number_of_events, Weight1, Weight2) = Str :-
> +	Weight1Str = int_to_string_thousands(Weight1),
> +	Weight2Str = int_to_string_thousands(Weight2),
> +	Str = "this node divides the suspect area into " ++
> +		"two regions of " ++ Weight1Str ++ " and " ++ Weight2Str ++
> +		" events each.".
> +
> +weighting_to_reason_string(suspicion, Weight1, Weight2) = Str :-
> +	Weight1Str = int_to_string_thousands(Weight1),
> +	Weight2Str = int_to_string_thousands(Weight2),
> +	Str = "this node divides the suspect area into " ++
> +		"two regions of suspicion " ++ Weight1Str ++ " and 
> +		" ++ Weight2Str ++ ".".

Why "each" for number_of_events but not for suspicion?

> +			list.append(!.Data, 
> [int_to_string_thousands(Weight)],
> +				!:Data)

We have "list.cons(int_to_string_thousands(Weight), !Data)" to add stuff
to the front of !Data; maybe we need a similar library predicate for adding
stuff at the end.

> +:- pragma export(
> +	mdb.declarative_debugger.suspicion_divide_and_query_search_mode = 
> out, +	"MR_DD_decl_suspicion_divide_and_query_search_mode").
> +

More funny indentation.

> +:- pred initialise_search_space(S::in, maybe(weighting_heuristic)::in, 
> T::in, +	search_space(T)::out) is det <= mercury_edt(S, T).

And even more.

> +:- type weighting_heuristic
> +	--->	number_of_events
> +	;	suspicion.
> +
> +	% Recalculate the value of the `weight' fields for all suspects in 
> the
> +	% search space if the given weighting heuristic is different from the
> +	% previous one.

And even more.

>  empty_search_space = search_space(no, no, counter.init(0), 
>  counter.init(0), -	map.init, bimap.init).
> +	map.init, bimap.init, no).

And more, of a different kind.

>  :- pred calc_suspect_weight(S::in, T::in, maybe(list(suspect_id))::in,
>  	suspect_status::in, search_space(T)::in, int::out, int::out) 
> @@ -1189,39 +1219,50 @@
>  calc_suspect_weight(Store, Node, MaybeChildren, Status, SearchSpace, 
>  Weight,
>  		ExcessWeight) :-
>  	(
> -		( Status = correct
> -		; Status = inadmissible
> -		)
> -	->
> -		Weight = 0,
> -		ExcessWeight = 0
> -	;
> -		edt_weight(Store, Node, OriginalWeight, ExcessWeight),
> +		SearchSpace ^ maybe_weighting_heuristic = yes(Weighting),
>  		(
> -			MaybeChildren = no,
> -			Weight = OriginalWeight
> +			( Status = correct
> +			; Status = inadmissible
> +			)
> +		->
> +			Weight = 0,
> +			ExcessWeight = 0
>  		;
> -			MaybeChildren = yes(Children),
> -			list.map(lookup_suspect(SearchSpace), Children,
> -				ChildrenSuspects),
> -			ChildrenNodes = list.map(
> -				func(S) = N :- N = S ^ edt_node, 
> -				ChildrenSuspects),
> -			list.foldl2(add_original_weight(Store), 
> -				ChildrenNodes, 0, ChildrenOriginalWeight,
> -				0, ChildrenExcess),
> -			list.foldl(add_existing_weight, ChildrenSuspects, 0,
> -				ChildrenWeight),
> +			calc_weight(Weighting, Store, Node, OriginalWeight, 
> +				ExcessWeight),

Changes like that that change levels of indentation are easier to review
when created with diff -b. Please post such a diff; I'll look at the rest
of it then.

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