This is an archive of the discontinued LLVM Phabricator instance.

Added std::assoc_legendre and std::legendre [sf.cmath]
Needs ReviewPublic

Authored by thebrandre on Apr 3 2019, 3:35 PM.

Details

Reviewers
None
Group Reviewers
Restricted Project
Summary

Added std::assoc_legendre and std::legendre from C++17, 29.9.5 Mathematical special functions [sf.cmath].

Resubmitted parts of Added implementations of five Mathematical Special Functions added in C++17
It is tested with a table of values generated by gcc-8.3.1 and boost-1.66.0 using boost::multiprecision::cpp_bin_float_oct (256bit float).

Notes:

  • The float version uses double internally because assoc_legendre will easily overflow for float. But it can be shown that assoc_legendre is bound by a value less than std::numeric_limits<double>::max() for the whole range required by the standard, x in [-1,1], l < 128, and m < 128. (There is a unit test for that and references to the literature in the code.)

So that seemed to be kind of a natural choice to me. Analog to Added std::assoc_laguerre and std::laguerre and boost, the double version is also promoted long double for internal calculations.

  • The implementation is inline for the same reason as D60869.

Related revisions:

Diff Detail

Event Timeline

thebrandre created this revision.Apr 3 2019, 3:35 PM
thebrandre added a reviewer: Restricted Project.Apr 3 2019, 3:35 PM

Thanks for doing all this. I'm not really ignoring these, it just seems like it. :-(

No worries. :-)
I know that this is time-consuming and I am well aware that is not a high-priority feature. And now I had time to reconsider and find my own mistakes. ;-)
The integration should be now much cleaner than my first draft.

Btw .... I should also point out that this draft implicitly checks the flag _LIBCPP_NO_EXCEPTIONS (via __throw_domain_error) in the src folder. I keep wondering which of these #defines may actually be used in cpp files. Should this be avoided so there's no reason to build different versions of the library?

thebrandre edited the summary of this revision. (Show Details)

To avoid AVX-SSE transition penalties (and for simplicity) the implementation is now inline.
See Added std::assoc_laguerre and std::laguerre.

I am quite certain by now that the benefits of the inline implementation outweigh the drawbacks ... but I am open for discussion/advice.

I recently became aware of the fact that gcc and msvc don't aggree on "Exceptions or Error Codes?" (Section III.D in N1542)

N1542 is in favor of signaling domain errors via

#include <cerrno>
errno = EDOM;

for consistency with other functions from cmath such as the log function and for compatibility with C.

VS2019's implementation does exactly that whereas gcc throws an std::domain_error. (As an aside: the exceptions of legendre got removed in gcc-9 but laguerre still throws for x<0 .)

The C++17 standard and N3060 simply refer to it as "domain error". Unfortunately, this term is ambiguous because both EDOM and std::domain_error are known by this name.

As N1542 suggests to use errno for perfectly valid reasons, I am inclined to change my patches accordingly even though it means that we'd diverge from libstdc++ in that matter.
Maybe/hopefully someone can help out with any (official) statements that give clearer directions?

Interestingly, boost is in-between: when you use boost::math::policies::errno_on_error instead of the default policy, you'll get close to MSVC's behavior. The default policy throws std::domain_error.

I'd also like to point out that the situation is even worse than that. For example, log(-1) not only sets errno but also raises a floating-point exception as if

std::feraiseexcept(FE_INVALID);

were called. This is also referred to as a "domain error".

We could mimic the behavior of the log function by introducing something like the following:

#include <cerrno>
#include <cmath>
#include <cfenv>

void __raise_math_domain_error() _NOEXCEPT {
#if math_errhandling & MATH_ERREXCEPT == MATH_ERREXCEPT
  std::feraiseexcept(FE_INVALID);
#endif
#if math_errhandling & MATH_ERRNO == MATH_ERRNO
  errno = EDOM;
#endif
}

Let's call that one "errno + floating-point exception".

However, neither boost nor msvc seem to go beyond "errno".
So when it comes down to the question "std::domain_error, errno, or 'errno + floating-point exception'?", I personally think errno is the way to go ...
but I would be sooooo happy for some feedback on that. ;-)

thebrandre edited the summary of this revision. (Show Details)

After a vivid discussion with myself, I went with "errno + floating-point exception".
In fact, the C11 Standard has a clear definition of a domain error in 7.12.1.
I added cmath.cpp with a definition of a function that sets the domain error as specified (similar to the comment above, just minus the bugs). It is not inline to avoid an include of cfenv and cerrno.

Note: the tests on invalid arguments only check for errno because clang doesn't support the STDC FENV_ACCESS pragma.

Also some minor changes:

  • Moved the tests for consistency with the latest changes in cmath (lerp).
  • Simplification of the tests: now using only one set of reference values generated by boost with multiprecision floats (256bit rounded to 128bit). I am pretty optimistic that every digit is correct, so that the comparison to other implementations no longer serves any purpose.
  • Rebased against current master branch.
Herald added a project: Restricted Project. · View Herald TranscriptMay 9 2023, 2:11 PM