[m-rev.] Re: [m-dev.] for review : quickcheck

[Mark Anthony BROWN]

Hi,

First a couple of general remarks.

	- A "human readable" diff would be far better for reviews (i.e.,
	  one in unified format with some context).
	- You still have not addressed my comment about the assumptions
	  in gen_arg that should be documented in the interface of the
	  module.

After you have addressed the points below, please post a relative diff.
Note that the mailing lists have changed slightly, and you should post
it to mercury-reviews.  (I have CC'd this to you just in case.)

Cheers,
Mark.

Xiao Chun Simon MEI writes:
> Hi,
> 
> qcheck.m was the only file I modify this round, so below
> is diff between the 2 versions of qcheck.m. But I did paste 
> over the log message, and changed the comment for rnd.m as David 
> suggested.
> 
> I've attached the new version of qcheck.m just in case the 
> relative diff is not enough.
> 
> For Mark to review.
> 
> 
> 
> Estimated hours taken : 185
> 
> qcheck is an autotesting tool similar to Haskell's Quickcheck.
> 
> RELEASE_NOTES:
> compiler/notes/authors.html:
> extras/README:
>         Modified to mention quickcheck.
> 
> extras/quickcheck/qcheck.m:
>         New file that contains the source code for qcheck.
> 
> extras/quickcheck/rnd.m:
>         New file written by conway. Its functions are similar 
>         to those in library random.m. The random numbers generated
> 	with random.m functions show strong correlation in lower
> 	bits; rnd.m does not seem have such problems.
> 
> 
> 
> 
> 
> 40,42c40,42
> < 	% Each distribution store the value in univ and it's number of 
> < 	% occurences in the second element.
> < :- type distribution == {univ, int}.
> ---
> > 	% Each distributions store the value in univ and its number of 

s/distributions store/distribution stores/

> > 	% occurences in the first element.
> > :- type distribution == {int, univ}.
> 78c78
> < 	%	followed by a Specific Frequency, a list of Generanl frequency,
> ---
> > 	%	followed by a Specific Frequency, a list of General frequency,
> 157a158,168
> > 	%	
> > 	% Quickcheck is able to generate random values for each input 
> > 	% argument of the invariant function at run time, provided 
> > 	% that there is a default/custom generator for that type. 
> > 	% The default generator is able to generate type int, char, 
> > 	% float, string, string, discriminated union and certain functions.
> > 	% Details are explained in the tutorials.
> > 	%
> > 	% Quickcheck makes the assumption that the distincation between
> > 	% a function and a discriminated union is that num_functors(FuncType) 
> > 	% is -1, while num_functors(UnionType) is not -1.
> 197c208
> < 	% functions with arity 0 to 10, it will throw an error is unable to
> ---
> > 	% functions with arity 0 to 10, it will throw an error if unable to
> 277c288
> < 	% qcheck//6 first seed the random number on local time, then it  
> ---
> > 	% qcheck/8 first seed the random number on local time, then it  
> 282,287c293,294
> < 	time__time(Sometime),
> <         { tm(Seconds, Minutes, Hours, _Weekday, Yearday, _Month, Year, _DST) 
> < 	  = time__localtime(Sometime) },
> < 	{ TotalSecs = ((( Year * 365 + Yearday ) * 24 + Hours ) * 60 + Minutes ) * 60 
> < 			 + Seconds },
> < 	{ init_setup(TotalSecs, RS0) },
> ---
> > 	time__time(CurrentTime),
> > 	{ init_setup(generate_seed_from_time(CurrentTime), RS0) },
> 305,306c312,313
> < 	    	{ list__sort(rev_field(Distributions), Distributions_Sorted) },
> < 	    	show_dist(rev_field(Distributions_Sorted)),
> ---
> > 	    	{ list__sort(Distributions, Distributions_Sorted) },
> > 	    	show_dist(Distributions_Sorted),

Are you sure this does what you meant it to?  I thought you wanted a
different ordering than the default, which means that you should call
the other version of list__sort, and pass a comparison predicate as the
first argument.  The comparison predicate that you write should compare
the second tuple argument before comparing the first argument if
necessary.

Oh, I see below that you have changed the representation so that the
default ordering does the intended thing.  This should be ok, then.

> 310d316
> < 
> 312a319,333
> > 	% generate_seed_from_time/1 converts the input time in calendar time
> > 	% format to the number of seconds from the beginning of current year.
> > 	% The numbers of seconds is reduced to an appropriate range for int to
> > 	% avoid overflow.
> > :- func generate_seed_from_time(time_t) = int.
> > :- mode generate_seed_from_time(in) = out.
> > generate_seed_from_time(CurrentTime) =  Seed :-
> >         tm(Seconds, Minutes, Hours, _Weekday, Yearday, _Month, Year, _DST) 
> > 	  = time__localtime(CurrentTime),
> > 	TotalSecs = ((( integer(Year) * integer(365) + integer(Yearday)) 
> > 		      * integer(24) + integer(Hours)) 
> > 		      * integer(60) + integer(Minutes)) 
> > 		      * integer(60) + integer(Seconds),
> > 	Seed = int( TotalSecs mod integer(max_int + 1) ).

Unfortunately, this won't work either, because (max_int + 1) is
undefined due to an overflow.  Is there a bug in the suggested code I
posted in the previous review?  If so, please tell me what it is!

> > 
> 388,389c409,410
> < 	% update_dist/3 recursively exacts the 'flag:info(univ)' from property i
> < 	% iand adds it to the master list of distribution.
> ---
> > 	% update_dist/3 recursively exacts the 'flag:info(univ)' from property
> > 	% and adds it to the master list of distribution.
> 407c428
> < update_dist_2(Univ, [], [{Univ, 1}]).
> ---
> > update_dist_2(Univ, [], [{1, Univ}]).
> 409c430
> < 	Distribution = {Patten, Counter},
> ---
> > 	Distribution = {Counter, Patten},
> 412c433
> < 	 	Output = [{Univ, Counter + 1} | Distributions]
> ---
> > 	 	Output = [{Counter + 1, Univ} | Distributions]
> 427c448
> < 	% test//5 then calls gen//4 to generate the arguments, the invariant
> ---
> > 	% test/7 then calls gen/6 to generate the arguments, the invariant
> 429,430c450
> < 	% the invariant test funtion with the 
> < 
> ---
> > 	% The test result is returned along with the arguments generated.
> 738c758
> < 	(if	find_user_gen(Datatype, UserGenerators, User_generator)
> ---
> > 	(if	find_user_gen(Datatype, UserGenerators, UserGenerator)
> 740,741c760,761
> < 		Univ = User_generator(Datatype, Frequencys, GF, UserGenerators,
> < 				      RS0,RS)
> ---
> > 		Univ = UserGenerator(Datatype, Frequencys, GF, UserGenerators,
> > 				      RS0, RS)
> 871,881c891,901
> < 	rnd__irange(1, const_million, X0,  RS0, RS1),
> < 	rnd__irange(1, const_million, X1,  RS1, RS2),
> < 	rnd__irange(1, const_million, X2,  RS2, RS3),
> < 	rnd__irange(1, const_million, X3,  RS3, RS4),
> < 	rnd__irange(1, const_million, X4,  RS4, RS5),
> < 	rnd__irange(1, const_million, X5,  RS5, RS6),
> < 	rnd__irange(1, const_million, X6,  RS6, RS7),
> < 	rnd__irange(1, const_million, X7,  RS7, RS8),
> < 	rnd__irange(1, const_million, X8,  RS8, RS9),
> < 	rnd__irange(1, const_million, X9,  RS9, RS10),
> < 	rnd__irange(1, const_million, X10, RS10, RS),
> ---
> > 	rnd__irange(1, max_int_argument, X0,  RS0, RS1),
> > 	rnd__irange(1, max_int_argument, X1,  RS1, RS2),
> > 	rnd__irange(1, max_int_argument, X2,  RS2, RS3),
> > 	rnd__irange(1, max_int_argument, X3,  RS3, RS4),
> > 	rnd__irange(1, max_int_argument, X4,  RS4, RS5),
> > 	rnd__irange(1, max_int_argument, X5,  RS5, RS6),
> > 	rnd__irange(1, max_int_argument, X6,  RS6, RS7),
> > 	rnd__irange(1, max_int_argument, X7,  RS7, RS8),
> > 	rnd__irange(1, max_int_argument, X8,  RS8, RS9),
> > 	rnd__irange(1, max_int_argument, X9,  RS9, RS10),
> > 	rnd__irange(1, max_int_argument, X10, RS10, RS),
> 1079,1089c1099,1109
> < 	rnd__irange(1, const_million, Seed_initial,      RS0, RS1),
> < 	rnd__irange(1, const_million, Seed_any_to_int1,  RS1, RS2),
> < 	rnd__irange(1, const_million, Seed_any_to_int2,  RS2, RS3),
> < 	rnd__irange(1, const_million, Seed_any_to_int3,  RS3, RS4),
> < 	rnd__irange(1, const_million, Seed_any_to_int4,  RS4, RS5),
> < 	rnd__irange(1, const_million, Seed_any_to_int5,  RS5, RS6),
> < 	rnd__irange(1, const_million, Seed_any_to_int6,  RS6, RS7),
> < 	rnd__irange(1, const_million, Seed_any_to_int7,  RS7, RS8),
> < 	rnd__irange(1, const_million, Seed_any_to_int8,  RS8, RS9),
> < 	rnd__irange(1, const_million, Seed_any_to_int9,  RS9, RS10),
> < 	rnd__irange(1, const_million, Seed_any_to_int10, RS10, _),
> ---
> > 	rnd__irange(1, max_int_argument, Seed_initial,      RS0, RS1),
> > 	rnd__irange(1, max_int_argument, Seed_any_to_int1,  RS1, RS2),
> > 	rnd__irange(1, max_int_argument, Seed_any_to_int2,  RS2, RS3),
> > 	rnd__irange(1, max_int_argument, Seed_any_to_int3,  RS3, RS4),
> > 	rnd__irange(1, max_int_argument, Seed_any_to_int4,  RS4, RS5),
> > 	rnd__irange(1, max_int_argument, Seed_any_to_int5,  RS5, RS6),
> > 	rnd__irange(1, max_int_argument, Seed_any_to_int6,  RS6, RS7),
> > 	rnd__irange(1, max_int_argument, Seed_any_to_int7,  RS7, RS8),
> > 	rnd__irange(1, max_int_argument, Seed_any_to_int8,  RS8, RS9),
> > 	rnd__irange(1, max_int_argument, Seed_any_to_int9,  RS9, RS10),
> > 	rnd__irange(1, max_int_argument, Seed_any_to_int10, RS10, _),
> 1196c1216
> < 	% prints out the elements in that tuple.
> ---
> > 	% print out the elements in that tuple.
> 1219c1239
> < 	{ Dist = {Univ, Freq} },
> ---
> > 	{ Dist = {Freq, Univ} },
> 1242c1262
> < 			error("sum_freq/6 : get_functor_ordinal/3 failed")
> ---
> > 			error("get_freq/5 : get_functor_ordinal/3 failed")
> 1362,1369c1382,1384
> < :- func rev_field(list({T1, T2})) = list({T2, T1}).
> < :- mode rev_field(in) = out is det.
> < rev_field([]) =  [].
> < rev_field([{A, B} | Xs]) = [{B, A} | rev_field(Xs) ]. 
> < 
> < :- func (const_million) = int.
> < :- mode (const_million) = out is det.
> < (const_million) = 1000000.
> ---
> > :- func (max_int_argument) = int.
> > :- mode (max_int_argument) = out is det.
> > (max_int_argument) = 1000000.
> 
> 
> 
> 
> 
> 
Content-Description: 

[Attachment, skipping...]

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