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.

Sign up for GitHub

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

Jump to bottom

Update t_test.py #242

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Update t_test.py #242

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions python/ml4ir/base/stats/t_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -458,6 +458,16 @@ def compute_required_sample_size(mean1, mean2, var1, var2, statistical_power, pv
if denominator == 0:
return np.inf
d = np.abs(float(mean1) - float(mean2)) / denominator
'''
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor Questions:

  • Should we be using " instead of ' as in the docstring?
  • There seems to be an extra indent in the details compared to the '''
  • Please start with capital (For)
  • Typos: temporeary

Main questions:

  • How is 2.24 calculated?
  • What's the plan to actually fix the issue and remove this temporary fix? Is there one?
  • power==0.9 and pvalue==0.1 are very specifix. Should we have something more generic e.g., power>=0.9 and pvalue<=0.1?

Thanks for catching this and trying to fix it here. Is there already an open issue on power_ttest?

for high d values the call to power_ttest will invoke the c++ boost lib that will throw an exception (below) that will not be caught here.
We can temporeary circumvent this behavior by saturating the d value at 4 (for statistical_power == 0.9 and pvalue == 0.1) and return
a pre-computed required sample size for this value.

Error in function boost::math::itrunc<double>(double): Value 5959970140.0539618 can not be represented in the target integer type.
terminate called after throwing an instance of 'boost::wrapexcept<boost::math::rounding_error>
'''
if d > 4 and statistical_power == 0.9 and pvalue == 0.1:
return 2.24
req_sample_sz = power_ttest(d, n, statistical_power, pvalue, contrast=typ, alternative=alternative)
return req_sample_sz
except:
Expand Down