From: Rich Felker <dalias@libc.org>
To: Damian McGuckin <damianm@esi.com.au>, MUSL <musl@lists.openwall.com>
Subject: Re: [musl] catan(z)
Date: Wed, 14 Aug 2024 21:16:56 -0400 [thread overview]
Message-ID: <20240815011655.GN10433@brightrain.aerifal.cx> (raw)
In-Reply-To: <20240812090022.GA3766212@port70.net>
On Mon, Aug 12, 2024 at 11:00:22AM +0200, Szabolcs Nagy wrote:
> * Damian McGuckin <damianm@esi.com.au> [2024-08-12 13:56:18 +1000]:
> > On Mon, 12 Aug 2024, Damian McGuckin wrote:
> >
> > > There is some argument that if you handle the special cases at infinity
> > > separately (which I think MUSL should do but I do not have time at the
> > > moment), then one can assume that because pi/2 is irrational, then one
> > > should never have to deal with the end points in the chunk of code where
> > > those two lines of code seen above should appear. I will have a chat
> > > sometime with the guy who wrote that logic in a WG14 paper when I get a
> > > really clear head and can line him up.
> >
> > Consider
> >
> > atan2(y, x)
> >
> > For any finite y and finite non-zero x floating point number arguments, i.e.
> > rational numbers, the result of atan2(y, x) must be rational and so is never
> > +/- pi (which is irrational and only occurs when the ration y/x is a
> > mathematical infinity, not an overflowing infinity). So, we can ignore the
> > endpoints as long as our special case handling takes care of the case of
> > zero x.
> >
> > I think that is correct .... or is my brain still not working properly after
> > too many late nights watching the Olympics.
>
> well atan2 cannot return exact +-pi, but one would still
> expect the right sign for the quadrant (y=+-0, x<0 case
> or finite y, x=-inf case)
>
> and then _redupi uses (int)(t/pi+-0.5) where that /pi is
> not exact the same way the +-pi returned from atan2 was
> not exact.
>
> i.e currently in the atan2==+-pi case the sign gets flipped
> by _redupi (even though double pi < exact pi so no need to
> reduce)
>
> i doubt the sign flip is needed, but even if that's the
> right result there are better ways to handle that case.
If I'm following, it sounds like the current behavior is either a
no-op or wrong and removing the _redupi step entirely would make it
correct or at least closer to correct. Do you agree?
Any suggestions for a test to run? I did some basic smoke testing for
large positive real inputs and got expected results.
Rich
next prev parent reply other threads:[~2024-08-15 1:17 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-08-11 14:01 Damian McGuckin
2024-08-11 20:08 ` Szabolcs Nagy
2024-08-12 3:39 ` Damian McGuckin
2024-08-12 3:56 ` Damian McGuckin
2024-08-12 9:00 ` Szabolcs Nagy
2024-08-15 1:16 ` Rich Felker [this message]
2024-08-15 2:12 ` Damian McGuckin
2024-08-15 14:28 ` Rich Felker
2024-08-16 7:26 ` Szabolcs Nagy
2024-08-16 8:18 ` Eric Pruitt
2024-08-15 13:18 ` Morten Welinder
2024-08-15 13:44 ` Rich Felker
2024-08-16 1:47 ` Damian McGuckin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240815011655.GN10433@brightrain.aerifal.cx \
--to=dalias@libc.org \
--cc=damianm@esi.com.au \
--cc=musl@lists.openwall.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.vuxu.org/mirror/musl/
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).