-
Notifications
You must be signed in to change notification settings - Fork 23
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
T-test does not work for single magnitude bin #140
Comments
Hi @khawajasim! Good find on this issue. I'll work on a fix for this tomorrow. Given the implementation right noow, we'd expect that bin to be [5.95, inf) instead of [5.95, 6.05]. Do you want to be able to specify a finite bin-width if there is only a single bin? In other words, It seems if we are treating that bin as open, we'd want to just check for that case and return index=0 if tthe magnitude >= 5.96. Do you agree? |
Hi Bill, Thank you for your response. Since I am just playing around with actual or dummy data, so I put all the magnitudes in just one bin, say 5.95 etc. And I had only one bin for all the data, so I just putting dh = 0.1 as a quick fix, which worked for the time being. But it may not be a robust solution for someone else with single bin. So yes better solution is [5.95, inf). |
Sounds good @khawajasim. I'm working on a fix for this right now. I'll ask you to review the code before I merge into master. |
Sure. |
Hi,
I noticed that T-test does not work for single magnitude bins. E.g. mbins = [5.95].
The error is traced down to "target_event_rates --> get_rates --> get_magnitude_index --> bin1d_vec" to line 75: h = bins[1] - bins[0].
Since there is only one bin, so bins[1] gives error. For now, I manually put h=0.1 in my copy of repo and its working.
Perhaps, as a solution, a check can be put here, that if there size(bin)==1, then default h=0.1. It is already taking last bins as open. So it should work.
Cheers,
Thanks
The text was updated successfully, but these errors were encountered: