[m-rev.] for review: float__pow: accept negative exponent; Russian peasants alg

[Fergus Henderson]

On 27-Feb-2002, Peter Moulder <pmoulder at csse.monash.edu.au> wrote:
> This diff has two parts: one is just to implement the russian peasants
> algorithm (O(lg Exp) instead of current O(Exp); comments in the code
> suggest that int__mod etc. weren't available when float__pow was written);

That part is fine.

> the other change involves an interface change, in that I'm proposing
> accepting negative exponents instead of either throwing an exception
> (current CVS version) or calling error/1 (0.10.1 version).

That part is fine.

The only bit which might be controversial is the behaviour of 0^0.
Note that there was a long discussion about the behaviour of
`float__pow(0.0, 0)' and `math__pow(0.0, 0.0)' on the Mercury mailing
lists.  The end result of that discussion was that we agreed that
`float__pow(0.0, 0)' should be defined to return 1.0.
In fact, I wrote a change to document that, and posted it for review,
but no-one reviewed it and I never got around to committing it.

> In any case, the change of algorithm is orthogonal to any change in
> interface, and I don't expect any issues with that part.  I've done
> some private testing of the change (source code available on request),
> though I haven't added anything to tests/general/float_test.m, which
> looks as if it's in a state of flux (most of it being commented out).

According to the cvs log messages, the parts of float_test.m which were
commented out were commented out in 1996 because they didn't work with
NU-Prolog.  We're no longer using NU-Prolog, so that is no longer an issue.
I will fix the test case.

> +++ library/float.m	27 Feb 2002 07:15:29 -0000
> +float__pow(Base, Exp) = Ans :-
> +	( Exp < 0 ->
> +		% Old behaviour (since 2 September 2001 in CVS):
> +		%throw(math__domain_error("float__pow"))
> +		% Behaviour before 2Sep, including in version 0.10.1:
> +		%error("float__pow taken with exponent < 0\n")
> +
> +		% Arguments in favour of retaining old behaviour:
> +		%   - Almost none that I can think of.  Even backwards compatibility
> +		%     isn't really an issue given that the program simply aborted
> +		%     in the last numbered-release version of the library.

After a certain point, too many comments can obscure the code.
These comments, I think, fall into that category; they are better
put in the CVS log message.

> +		% Arguments in favour of allowing negative exponent:
> +		%   - There's no mathematical reason not to allow negative
> +		%     exponent (except perhaps for the Base=+/-0.0 case,
> +		%     see above).
> +		%   - If we're doing a check for negative Exponent anyway
> +		%     then there's no efficiency reason not to handle
> +		%     negative exponents.
> +		%     (Note that at least until now the code hasn't
> +		%     even called domain_checks before checking for
> +		%     negative exponent.)
> +		%   - It's useful.  (Especially for Base = 10.0 or 2.0.)
> +		%   - It seems strange that `<<' in Mercury accepts a negative
> +		%     right operand when float__pow doesn't.

These comments are useful.  If you delete the earlier comments,
start these with e.g.
		% Note that we allow negative exponents.
		% These are allowed because:

Another important reason is "for conformance with LIA-2
(the language-independent arithmetic standard, part 2)".

It's not clear what "see above" refers to here.

> +		% Alternatives are to use the old behaviour except also
> +		% test domain_checks; and/or to delay making a decision so
> +		% that more people can see and add to this discussion before
> +		% we change the behaviour.
> +		%
> +		% (If we decide to reject or delay allowing float__pow for
> +		% negative exponents, then consider adding a ldexp function
> +		% to math.m or equivalently a `<<' operator to float.m; these
> +		% at least address the case where Base is a constant 2.0.)

Those comments I think would be better put in the CVS log message.

So, apart from being IMHO a bit over-verbosely commented, this change is
fine.

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