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

Update control$df to control$power_df in simsum.R #33

Merged
merged 1 commit into from
Jul 5, 2021

Conversation

Kaladani
Copy link
Contributor

@Kaladani Kaladani commented Jul 4, 2021

Thank you for your great package.
When applying it and comparing it to the results of my own code, I got different power/rejection-rates and checked your code. Your check of the control list seems to have a small error due to changes in names. Your checking if control$df is a number instead of control$power_df. This means that you cannot specify power_df at the moment. It's either set to the default NULL when you specify power_df in the control list or if you specify df it's returning an error for the name-control of the list.

Thank you for your great package. 
When applying it and comparing it to the results of my own code, I got different power/rejection-rates and checked your code. Your check of the control list seems to have a small error due to changes in names. Your checking if control$df is a number instead of control$power_df. This means that you cannot specify power_df at the moment. It's either set to the default NULL when you specify power_df in the control list or if you specify df it's returning an error for the name-control of the list.
@Kaladani Kaladani changed the title Update simsum.R Update control$df to control$power_df in simsum.R Jul 4, 2021
@ellessenne
Copy link
Owner

Thanks very much for your PR, good catch!

@ellessenne ellessenne merged commit 8c4e6de into ellessenne:master Jul 5, 2021
ellessenne added a commit that referenced this pull request Jul 5, 2021
ellessenne added a commit that referenced this pull request Jul 5, 2021
@Kaladani
Copy link
Contributor Author

Kaladani commented Jul 5, 2021

I'm glad I could help. As an idea for the the next update (post 0.1.0) you might want to consider to allow to add a column of p-values by the user. This way, power could be calculated even if the model is using neither z- nor t-test. This might be particularly useful to allow for e.g. rejection rate of model fit and comparison tests. It would also align stronger to Morris et. a. 2019 definition of power/rejection estimation.

@ellessenne
Copy link
Owner

Thanks for the suggestion, I'll definitely consider this for the next version of {rsimsum} – I opened a new issue (#35) if you want to keep track of it, or add specific examples.

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.

None yet

2 participants