rOpenSci reviewer feedback from @jcavieresg

RWParsons commented 12 months ago

Review Comments

The package 'GLMMcosinor' is very well designed and aligned with the conventional syntax employed in creating statistical models (e.g. $\texttt{lm()}$ or $\texttt{glm()}$ functions). This makes it easy to comprehend the primary function, $\texttt{cglmm()}$.
The package also allows a fast computation considering complex models (as mixed models).
The documentation associated with the package is intuitive and user-friendly, making it easy to follow. As a result, nearly all the provided examples can be successfully replicated.
The visual representation generated by the $\texttt{autoplot()}$ function is a valuable feature, eliminating the need for additional coding.

Suggestions

Is it possible to set $\texttt{Hessian = TRUE}$ in the $\texttt{cglmm()}$ function? This could be valuable for conducting diagnostics on the model. It should not be a problem if the authors are using TMB/glmmTMB as a precursor to the package.
Providing a brief documentation of the underlying theory behind the package could be valuable. This is because TMB/glmmTMB relies on maximum likelihood for estimating models without random effects, while it employs Laplace approximation when random effects are declared in the model. While referencing relevant literature is often sufficient, offering a concise explanation in the documentation of this package could serve as an additional helpful resource.
For dispersion models, like Poisson or Negative Binomial for the response variable, it might be beneficial to include a "dispersion statistic" for comparative purposes.

Minor suggestions

It might be better to use the %>% operator (pipe) instead of $|>$ for the reproducible examples of mixed models. I encountered some difficulties when attempting to reproduce them.
The same when the authors are declaring as 'factor' the variable 'subject'. I used dat_mixed$subject <- as.factor(dat_mixed$subject) and it worked.

RWParsons commented 11 months ago

Suggestions and how they've been addressed:

[x] Is it possible to set $\texttt{Hessian = TRUE}$ in the $\texttt{cglmm()}$ function? This could be valuable for conducting diagnostics on the model. It should not be a problem if the authors are using TMB/glmmTMB as a precursor to the package.

afaik, this is controlled by the control argument when calling glmmTMB() - if this is the case, then the user can control it when calling cglmm as all additional arguments (...) are passed to the glmmTMB(...) call here.
[x] Providing a brief documentation of the underlying theory behind the package could be valuable. This is because TMB/glmmTMB relies on maximum likelihood for estimating models without random effects, while it employs Laplace approximation when random effects are declared in the model. While referencing relevant literature is often sufficient, offering a concise explanation in the documentation of this package could serve as an additional helpful resource.

Have updated the README (thanks @oliverjayasinghe).
[x] For dispersion models, like Poisson or Negative Binomial for the response variable, it might be beneficial to include a "dispersion statistic" for comparative purposes.

Have created a stats::sigma() method (cglmm.sigma()) that calls glmmTMB::sigma() with the underlying glmmTMB object.

Minor suggestions

[x] It might be better to use the %>% operator (pipe) instead of $|>$ for the reproducible examples of mixed models. I encountered some difficulties when attempting to reproduce them.

Have removed all uses of the base R pipe (|>) in the package.
[x] The same when the authors are declaring as 'factor' the variable 'subject'. I used dat_mixed$subject <- as.factor(dat_mixed$subject) and it worked.

Have updated these in the tests - please let me know whether this works where you had found this issue?

RWParsons commented 10 months ago

ropensci / GLMMcosinor