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

Cin function #397

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Cin function #397

wants to merge 5 commits into from

Conversation

seadra
Copy link

@seadra seadra commented Apr 21, 2022

(Disclaimer: first time contributor to a Julia project.)

Added the Cin function, which is the other cosine integral function given by the integral of (1-cost)/t from 0 to x.
Approximation obtained using Mathematica's EconomizedRationalApproximation. Maximum error is 1.1e-10 in the range [0,16] for the input.

I named it cosint2. maybe cin is a better name?

@codecov
Copy link

codecov bot commented Apr 21, 2022

Codecov Report

Merging #397 (017be91) into master (ce58a13) will increase coverage by 1.62%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #397      +/-   ##
==========================================
+ Coverage   91.99%   93.61%   +1.62%     
==========================================
  Files          12       12              
  Lines        2809     2773      -36     
==========================================
+ Hits         2584     2596      +12     
+ Misses        225      177      -48     
Flag Coverage Δ
unittests 93.61% <100.00%> (+1.62%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/SpecialFunctions.jl 100.00% <ø> (ø)
src/chainrules.jl 100.00% <ø> (ø)
src/sincosint.jl 100.00% <100.00%> (+34.54%) ⬆️
src/beta_inc.jl 92.36% <0.00%> (-0.24%) ⬇️
src/gamma.jl 95.14% <0.00%> (+2.13%) ⬆️
src/gamma_inc.jl 93.46% <0.00%> (+3.45%) ⬆️

Continue to review full report at Codecov.

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

@stevengj
Copy link
Member

Maximum error is 1.1e-10 in the range [0,16] for the input.

This seems a little large. I would usually aim for a relative error < 1e-13 everywhere.

@seadra
Copy link
Author

seadra commented May 13, 2022

What are the expectations on relative error when the function value is 0?

@seadra
Copy link
Author

seadra commented Jul 7, 2022

I updated the function such that it relies on the existing cosint(x) everywhere except for the region with small |x| in which relative and absolute errors are better than about 1e-15 in my checks

@seadra
Copy link
Author

seadra commented Jul 7, 2022

Also, possibly for a different PR but cosint for negative values currently throws domain error, but it can be defined in terms of Cin(x): https://en.wikipedia.org/wiki/Trigonometric_integral#Cosine_integral

@stevengj
Copy link
Member

What are the expectations on relative error when the function value is 0?

When the function value is exactly zero, you want to also return 0. And where the function value is near zero, you still want low relative error. Often this means switching to a Taylor expansion in the vicinity of a root.

@seadra
Copy link
Author

seadra commented Jul 11, 2022

Often this means switching to a Taylor expansion in the vicinity of a root.

OK, that's what I did exactly. I assume the PR should be fine then.

One might want to tune the y < 0.5 line (the threshold value for |x| beyond which it falls back to existing cosint and log functions), although around |x| = 0.5, the rel and abs errors are still well below the threshold you mentioned.

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