Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#6171 closed Bugs (fixed)

nonconforming behavior of inverse functions on branch cuts

Reported by: Richard B. Kreckel <kreckel@…> Owned by: John Maddock
Milestone: To Be Determined Component: math
Version: Boost 1.48.0 Severity: Problem
Keywords: Cc:

Description

The inverse trigonometric and hyperbolic functions in boost/math treat signed zero incorrectly. The positions of the branch cuts are right, but C99 specifies to "map a cut so the function is continuous as the cut is approached coming around the finite endpoint of the cut in a counter clockwise direction" and so does C++11.

Here are some examples of failures:

  • asin(-1.75-0.0*I) returns -1.5708+1.1588*I (should be -1.5708-1.1588*I)
  • acos(-1.25+0.0*I) returns +3.1416+0.6932*I (should be +3.1416-0.6932*I)
  • atan(-0.0-1.75*I) returns +1.5708-0.6496*I (should be -1.5708-0.6496*I)
  • asinh(+0.0+1.5*I) returns -0.9624+1.5708*I (should be +0.9624+1.5708*I)
  • acosh(-2.5+0.0*I) returns +1.5668-3.1416*I (should be +1.5668+3.1416*I)
  • atanh(-2.0-0.0*I) returns -0.5493+1.5708*I (should be -0.5493-1.5708*I)

I'm attaching a patch.

Attachments (2)

boost.patch (3.1 KB ) - added by Richard B. Kreckel <kreckel@…> 11 years ago.
patch
acosh.patch (682 bytes ) - added by Richard B. Kreckel <kreckel@…> 11 years ago.

Download all attachments as: .zip

Change History (6)

by Richard B. Kreckel <kreckel@…>, 11 years ago

Attachment: boost.patch added

patch

comment:1 by Richard B. Kreckel <kreckel@…>, 11 years ago

Reading my report again, I appear to have quoted the wrong section of the standard. The one that applies is the first paragraph in [7.3.3] (not the second): "For implementations with a signed zero (including all IEC 60559 implementations) that follow the specifications of annex G, the sign of zero distinguishes one side of a cut from another so the function is continuous (except for format limitations) as the cut is approached from either side." I apologize for the confusion. But anyway: the patch I attached accomplishes exactly that and is correct. Could somebody, please, apply it?

comment:2 by anonymous, 11 years ago

Investigating: your patch doesn't quite get all the cases (and causes some tests to fail), but I think I'm nearly there....

John Maddock.

comment:3 by John Maddock, 11 years ago

Resolution: fixed
Status: newclosed

(In [75933]) Fix complex number routines to work with signed zeros, changes involve:

  • Use boost::math::signbit rather than comparison to zero.
  • Use boost::math::changesign rather than unary negation (unary negation fails for Intel on Linux when the argument is a zero).
  • Update to use boost::math::isnan/isinf rather than the old code.
  • Update to use boost::math constants.

Fixes #6171.

comment:4 by anonymous, 11 years ago

Thanks for applying this patch. I've tried it and it passes my tests.

I see only one potential problem in the way acosh has been fixed: it tests the sign of the imaginary part of the result of the acos function. It is generally much safer to test the location in the complex plain of input variables like z.imag() instead of the location of intermediate results like result.imag(). This is because intermediate results may suffer from roundoff.

My original patch avoided this problem I'm seeing problems related to this in various projects (e.g. GLibC http://cygwin.com/bugzilla/show_bug.cgi?id=13305). Please consider applying the patch I'm going to attach next.

by Richard B. Kreckel <kreckel@…>, 11 years ago

Attachment: acosh.patch added
Note: See TracTickets for help on using tickets.