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

Already on GitHub? Sign in to your account

BUG: limit input ranges in special.nctdtr #3222

Merged
merged 4 commits into from Feb 24, 2014

Conversation

Projects
None yet
4 participants
Contributor

richardtsai commented Jan 17, 2014

This PR is similar to #3153, and it will fix #2667. The limitations are described in the original comments.
This will also fix #3209 to some extent. There seems to be no efficient algorithm to calculate the PDF of the noncentral t distribution in high accuracy when the noncentrality parameter is large. The limitations will help avoid overflows and give a reasonable approximation under practical conditions. A warning will be issued in such cases as well.

Coverage Status

Coverage remained the same when pulling 738834d on richardtsai:nctdtr_inf_loop into 3b30d25 on scipy:master.

Owner

pv commented Jan 17, 2014

The usual approach in scipy.special in dealing with total loss of precision is to return nans.
This could also likely alert the user more effectively than warnings that something is wrong.

Contributor

richardtsai commented Jan 20, 2014

The range of pnonc is limited to [-1e4, 1e4]. But a _TimeoutError was raised in test_mpmath.TestSystematic.test_betainc on Travis CI. I have no idea about this and I cannot reproduce it.

@ev-br ev-br closed this Jan 31, 2014

@ev-br ev-br reopened this Jan 31, 2014

Member

ev-br commented Jan 31, 2014

close/reopen to ping TravisCI

Owner

pv commented Jan 31, 2014

@ev-br: Note that Travis-CI nowadays has a button for restarting the job, without need for close+reopen of the ticket.

Contributor

richardtsai commented Jan 31, 2014

Thanks! It passes now.

@pv pv added the PR label Feb 19, 2014

Member

ev-br commented Feb 23, 2014

Josef's example from #2667 still hangs for me with this PR.
The problem with insanely large non-centrality parameters, #3209 still seems to be there as well:

>>> stats.nct.cdf(45,9,65536) 
1.0

am tentatively adding the needs-work tag.

@ev-br ev-br added the needs-work label Feb 23, 2014

BUG: overflow in cumtnc.f
Fix an overflow bug in cumtnc.f. So the range of the nc parameter
is enlarged to [-1e6, 1e6]. However, a larger nc parameter may still
result in total loss of precision.
Contributor

richardtsai commented Feb 23, 2014

It works for me. Josef's example return nan as expected on my computer.
As for the large ncp problem, I've fixed the overflow bug in f0bb77d. But a very large nc parameters will result in total loss of precision, so the function will return a nan instead if the input nc parameter > 1e6.

In [3]: stats.nct._sf(np.nan, 125.531223551, 2.82324307702)
Out[3]: nan

In [4]: stats.nct.cdf(45,9,65536)
Out[4]: 0.0

In [5]: stats.nct.cdf(45,9,1000001)
Out[5]: nan

Travis-ci raised an error. It seems that there's something wrong with the networks.

Coverage Status

Coverage remained the same when pulling 2e58ebe on richardtsai:nctdtr_inf_loop into 3b30d25 on scipy:master.

Member

ev-br commented Feb 23, 2014

It now does indeed. With a clean rebuild the last commit works for me as well.
Looks good to me now.

@ev-br ev-br removed the needs-work label Feb 23, 2014

pv added a commit that referenced this pull request Feb 24, 2014

Merge pull request #3222 from richardtsai/nctdtr_inf_loop
BUG: special: limit input ranges in special.nctdtr

There seems to be no efficient algorithm to calculate the
PDF of the noncentral t distribution in high accuracy when
the noncentrality parameter is large. The limitations will help
avoid overflows and give a reasonable approximation under
practical conditions.

This also avoids the code getting stuck in an infinite loop.

@pv pv merged commit 265892c into scipy:master Feb 24, 2014

1 check passed

default The Travis CI build passed
Details
Owner

pv commented Feb 24, 2014

Thanks, LGTM

@pv pv added this to the 0.14.0 milestone Feb 24, 2014

@richardtsai richardtsai deleted the richardtsai:nctdtr_inf_loop branch Jun 3, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment