Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle single query on ellint_3 #1227

Closed
wants to merge 1 commit into from
Closed

Conversation

ckormanyos
Copy link
Member

Hi John (@jzmaddock),

So I'm boiling down the double-double backend. I was happily able to remove a few errors based on your advice so far.

I've come down to a small handful of queries. We need to go through these one at a time. I am a little bit shaky in this area. But I'll just express the query anyway.

In this PR, we find one single change on ellint_3 test data. I suspect this change is needed because we call ellint_3 with an infinite-parameter whereby there is no check for infinity other than a throw.

I see two choices here:

  • Add some more refined check-code to ellint_3
  • or avoid the test case for situations not able to handle the large order of the argument.

Your thoughts?

Cc: @mborland

Copy link

codecov bot commented Dec 20, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.71%. Comparing base (52b029f) to head (abc73f6).

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #1227      +/-   ##
===========================================
- Coverage    93.83%   93.71%   -0.13%     
===========================================
  Files          657      644      -13     
  Lines        55243    54138    -1105     
===========================================
- Hits         51838    50736    -1102     
+ Misses        3405     3402       -3     
Files with missing lines Coverage Δ
test/test_ellint_3.hpp 100.00% <ø> (ø)

... and 35 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 52b029f...abc73f6. Read the comment docs.

@jzmaddock
Copy link
Collaborator

I might need to look at this some more: the LDBL_MAX_EXP > 16382 check is there purely so we don't get compiler errors from an out-of-range literal when long double=double. Clearly this "works" when T=double and long double is an extended type, so it should work for double double too...

@jzmaddock
Copy link
Collaborator

OK, so we're calculating ellint_3(0, -INF,, 0.0009765625), and simulating your type approximately with cpp_bin_float:

using dd_t = boost::multiprecision::number<boost::multiprecision::debug_adaptor<boost::multiprecision::cpp_bin_float<106, boost::multiprecision::digit_base_2, void, int, std::numeric_limits<double>::min_exponent, std::numeric_limits<double>::max_exponent>>>;

I get the correct answer of 0.

It all comes down to:

return atan(vcr * tan(phi)) / vcr;

which evaluates as atan(INF)/INF which should be zero.

Where do you see things go wrong and/or throw for your type?

@ckormanyos
Copy link
Member Author

ckormanyos commented Dec 21, 2024

Where do you see things go wrong and/or throw for your type?

I'm working frantically on that.

It all comes down to: [line 145]

Um, I do not reach line 145. I return from the subroutine at line 71 with a domain error.

The check at line 68 tells me that (v > 0) is true since v is $\infty$. And the second logic check on line 68 (1 / v < (sphi * sphi)) also ends up being true for me since $1/{\infty}$ is zero and sphi is real and finite.

@ckormanyos
Copy link
Member Author

The check at line 68

In fact, I need to find out how the similarly configured cpp_bin_float type squeezes through that logic?. Or is it I, myself, as usual, making a blunder of assumption here?

@ckormanyos
Copy link
Member Author

Thinking a bit more, how can you get to line 145 if v is $\infty$? Shouldnt the logic check at line 142 (v < 1) be false if v is $\infty$?

@jzmaddock
Copy link
Collaborator

I have:

sphi = 0.000976562344779578298906910168119791
k = 0
v = -INF

So... the check at line 58 fails because k * k * sphi * sphi is zero.

And the check at line 68 also fails because v > 0 is false for v = -INF.

@ckormanyos
Copy link
Member Author

And the check at line 68 also fails because v > 0 is false for v = -INF.

Ah. My infinity is POSITIVE (as in +INF). I must have a critical error somewhere in the backend. I will find this now. Thank you John.

@ckormanyos
Copy link
Member Author

John (@jzmaddock) many thanks. The specfun tests are really good!

This test case revealed two critical errors in the preliminary cpp_double_fp_backend!

  • We were not retaining the negative sign for overly-large string read.
  • And another error was revealed! The backend had $\sqrt{\infty}={\text{NaN}}$, instead of $\infty$.

@ckormanyos
Copy link
Member Author

ckormanyos commented Dec 21, 2024

Clesed PR as error in client multiple-precision backend candidate. Thanks John AGAIN! (@jzmaddock).

Cc: @mborland

@ckormanyos ckormanyos closed this Dec 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants