[m-rev.] for review: fix float_to_string so that roundtripping works

[Fergus Henderson]

On 19-Nov-2002, Peter Ross <pro at missioncriticalit.com> wrote:
> Fix a bug where floats couldn't be roundtripped via a string.
> 
> library/string.m:
> 	When converting a string to a float check before returning the
> 	string that it can be roundtripped, if not then output it with
> 	an increased precision.

That looks like it might be a reasonable solution.

> tests/general/Mmakefile:
> tests/general/float_roundtrip.exp:
> tests/general/float_roundtrip.m:
> 	A test case.

Does this change cause regressions in any of the other test cases
in the Mercury test suite?

> Index: library/string.m
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/library/string.m,v
> retrieving revision 1.184
> diff -u -r1.184 string.m
> --- library/string.m	15 Nov 2002 04:50:37 -0000	1.184
> +++ library/string.m	19 Nov 2002 16:47:37 -0000
> @@ -1824,18 +1824,36 @@
>  %-----------------------------------------------------------------------------%
>  
>  :- pragma foreign_proc("C",
> -	string__float_to_string(FloatVal::in, FloatString::uo),
> +	string__float_to_string(Flt::in, Str::uo),
>  		[will_not_call_mercury, promise_pure, thread_safe], "{
> -	char buf[500];
> -	sprintf(buf, ""%#.15g"", FloatVal);
> -	MR_allocate_aligned_string_msg(FloatString, strlen(buf), MR_PROC_LABEL);
> -	strcpy(FloatString, buf);
> +#ifdef MR_USE_SINGLE_PREC_FLOAT
> +	const char *format_a = ""%.7g"";
> +	const char *format_b = ""%.9g"";
> +	const char *format_r = ""%f"";
> +	float round;
> +#else
> +	const char *format_a = ""%.15g"";
> +	const char *format_b = ""%.17g"";
> +	const char *format_r = ""%lf"";
> +	double round;
> +#endif

I'd be tempted to do

	MR_Float round;

outside the #ifdef.

The way we actually read floats (string__to_float, etc.)
is to scan them in to a double using "%lf", and then 
assign this double to a value of type MR_Float.
In the MR_USE_SINGLE_PREC_FLOAT case, this may produce different results
than scanning in directly as a float.  The code here should match
the technique used in string__to_float.  (Add comments mentioning this
in both places.)

If %.15g is not sufficiently precise, I think it would be better to try
%.16g before %.17g.  That avoids the problem of sometimes outputting more
precision than the float actually holds, which would be misleading.

Once you have three alternatives to try, this would be most elegantly
implemented in a loop, using the "%.*g" format which allows the precision
to be specified by a separate argument to printf.

> +	Str = MR_make_string(MR_PROC_LABEL, format_a, Flt);
> +
> +		/*
> +		 * Round-trip the float to ensure the precision was 
> +		 * sufficient, and if not try with the next precision.
> +		*/
> +	sscanf(Str, format_r, &round);

> +	if (round != Flt)
> +	{

The { should be on the same line as the `if'.

> -:- pragma foreign_proc("MC++",
> +:- pragma foreign_proc("C#",
>  	string__float_to_string(FloatVal::in, FloatString::uo),
>  		[will_not_call_mercury, promise_pure, thread_safe], "{
> -	FloatString = System::Convert::ToString(FloatVal);
> +	FloatString = FloatVal.ToString(""R"");
>  }").

What's that for?

That change was not mentioned in the log message,
and is not acceptable without some rationale.
Also, it would help to document what the "R" argument to ToString() means.

-- 
Fergus Henderson <fjh at cs.mu.oz.au>  |  "I have always known that the pursuit
The University of Melbourne         |  of excellence is a lethal habit"
WWW: <http://www.cs.mu.oz.au/~fjh>  |     -- the last words of T. S. Garp.
--------------------------------------------------------------------------
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
--------------------------------------------------------------------------