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

FIX: rank-size test by inc. sample size #556

Merged
merged 1 commit into from
Jul 1, 2020

Conversation

bktaha
Copy link
Contributor

@bktaha bktaha commented Jun 30, 2020

As mentioned in PR #551 the test for rank_size method checks for linearity in the output log-log plot. I did this by performing linear regression of log(size_data) on log(rank_data) and comparing the resulting r_squared value to 1.

Turns out an error tolerance of 1e-4 for a sample size of 1000 was a little optimistic. I have increased the sample size to 10000, reduced the tolerance to 1e-3 and the comparison works as expected now.

Another mistake was setting the seed for exponential draw but not the Pareto draw, I've fixed that as well. I'm not sure if setting a seed here is a good practice in the first place, if not I can remove it and increase sample size / reduce tolerance further.

Please let me know if this is an acceptable fix. This should close #553.

@mmcky mmcky requested a review from jstac June 30, 2020 23:22
@mmcky
Copy link
Contributor

mmcky commented Jun 30, 2020

thanks @bktaha for this well documented PR and for looking into the test failure. It is greatly appreciated.
@jstac would you mind to cast a quick eye over this if you have time?

@jstac
Copy link
Contributor

jstac commented Jul 1, 2020

Nice work @bktaha. Many thanks. This looks good to me @mmcky.

@mmcky
Copy link
Contributor

mmcky commented Jul 1, 2020

thanks @jstac

@mmcky mmcky merged commit b993db8 into QuantEcon:master Jul 1, 2020
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.

Issue with test for rank_size
3 participants