Re: Proposal: Trigonometric functions in degrees - Mailing list pgsql-hackers

On Tue, Nov 10, 2015 at 11:17 PM, Michael Paquier <michael.paquier@gmail.com> wrote:
> On Sun, Nov 1, 2015 at 9:34 PM, Dean Rasheed <dean.a.rasheed@gmail.com> wrote:
>> On 27 October 2015 at 08:24, Dean Rasheed <dean.a.rasheed@gmail.com> wrote:
>>> I think it's still feasible to have sind(30) = 0.5 exactly and keep
>>> monotonicity....
>>>
>>
>> Here's a patch along those lines. It turned out to be fairly
>> straightforward. It's still basically a thin wrapper on top of the
>> math library trig functions, with a bit of careful scaling to ensure
>> that curves join together to form continuous functions that are
>> monotonic in the expected regions and return exact values in all the
>> special cases 0,30,45,60,90,...
>>
>> I also modified some of the CHECKFLOATVAL() checks which didn't look
>> right to me, unless there's some odd platform-specific behaviour that
>> I'm not aware of, functions like sin and asin should never return
>> infinity.

-       CHECKFLOATVAL(result, isinf(arg1), true);
+       CHECKFLOATVAL(result, false, true);
        PG_RETURN_FLOAT8(result);

Hm. I would let them as-is, and update your patch to do the similar checks in the new functions introduced. See f9ac414 from 2007 which is the result of the following thread:
http://www.postgresql.org/message-id/200612271844.kBRIiVb18465@momjian.us
It doesn't seem wise to be backward regarding those Inf/NaN checks.

> The alternative expected outputs for test float8 need to be updated,
> this is causing regression failures particularly on win32 where 3
> digits are used for exponentials and where tan('NaN') actually results
> in an ERROR. See for example the attached regressions.diffs.

It would be nice to split the results specific to NaN and Infinity into separate queries. The test for arctangent is one where things should be splitted.

c5e86ea took some of the OIDs of this previous patch, so I rebased it as attached.

        result = 1.0 / result;
-       CHECKFLOATVAL(result, true /* cotan(pi/2) == inf */ , true);
+       CHECKFLOATVAL(result, true /* cot(0) == Inf */ , true);
        PG_RETURN_FLOAT8(result);
This one is true. it could be corrected as an independent fix.

+       if (isinf(arg1) || arg1 < -1 || arg1 > 1)
+               ereport(ERROR,
+                               (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
+                                errmsg("input is out of range")));
This should check on -1.0 and 1.0?

+       if (arg1 > 90)
+       {
+               /* tand(180-x) = -tand(x) */
+               arg1 = 180 - arg1;
+               sign = -sign;
+       }
Similarly the series of checks in atand, dcosd, asind, should use .0 precision points? Same remark for other code paths like cosd_0_to_60 for example.

+       if (arg1 > 180)
+       {
+               /* tand(360-x) = -tand(x) */
+        arg1 = 360 - arg1;
+               sign = -sign;
+       }
Picky detail: be careful of incorrect tab spaces.

=# select acos(-1.1);
 acos
------
  NaN
(1 row)
=# select acosd(-1.1);
ERROR:  22003: input is out of range
LOCATION:  dacosd, float.c:1752

I had as well a look at the monotony of those functions, using rough queries like this one, and things are looking nice. The precise values are taken into account and their surroundings are monotone.
with degrees as (
select generate_series(89.999999998, 90.000000002, 0.000000001)
union all select generate_series(44.999999998, 45.000000002, 0.000000001)
union all select generate_series(29.999999998, 30.000000002, 0.000000001)
union all select generate_series(-0.000000002, 0.000000002, 0.000000001)
union all select generate_series(59.999999998, 60.000000002, 0.000000001))
SELECT x, cosd(x), sind(x), tand(x) FROM degrees as deg(x);
with degrees as (
select generate_series((sqrt(3) / 3 - 0.00001)::numeric, (sqrt(3) / 3 + 0.00001)::numeric, 0.000001)
union all select generate_series((sqrt(3) / 2 - 0.00001)::numeric, (sqrt(3) / 2 + 0.00001)::numeric, 0.000001)
union all select generate_series(0.5 - 0.00001, 0.5 + 0.00001, 0.000001)
union all select generate_series(0, 0.00001, 0.000001)
union all select generate_series(0.99999, 1, 0.000001))
select x, acosd(x), asind(x), atand(x) from degrees as deg(x);
Attached are the results of all those things if others want to have a look.
Regards,
--
Michael