Re: Proposal: Trigonometric functions in degrees

From: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
To: Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Peter Eisentraut <peter_e(at)gmx(dot)net>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Proposal: Trigonometric functions in degrees
Date: 2015-11-11 06:04:40
Message-ID: CAB7nPqQ+KEtNXKUBNEzpOj4AESC8ipB3gUpgdbOb+b3KRgqzCw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Nov 10, 2015 at 11:17 PM, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
wrote:
> On Sun, Nov 1, 2015 at 9:34 PM, Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com>
wrote:
>> On 27 October 2015 at 08:24, Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)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
Some results are inconsistent, it seems that we should return NaN in the
second case instead of an error.

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

Attachment Content-Type Size
degrees.sql application/octet-stream 15.7 KB
20151111_trigd_v2.patch text/x-patch 36.7 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2015-11-11 06:55:02 Re: Dangling Client Backend Process
Previous Message Thomas Munro 2015-11-11 05:37:43 Proposal: "Causal reads" mode for load balancing reads without stale data