paul-buerkner / thurstonianIRT

Fit Thurstonian IRT models in R using Stan, lavaan, or Mplus
GNU General Public License v3.0
31 stars 12 forks source link

Weak Request: More comprehensive tests #8

Closed russellpierce closed 5 years ago

russellpierce commented 5 years ago

Consider this a weak request. You can tell me to fly a kite here - but I think considering this request may yield a stronger package that is easier/faster to give a positive review to.

I'm looking at the checklist item "Are there automated tests or manual steps described so that the function of the software can be verified?".

Your current {testthat} tests appear to mostly validate that the code executes without error. Have you considered building a vignette with a larger sim that demonstrates that the parameters you put into the simulation code can be recovered from the model results you provide (under each of the fitting engines)? A sufficiently stable sim may be computationally time consuming. However, vignettes aren't rebuilt by CRAN (http://r-pkgs.had.co.nz/vignettes.html). So, your patience is the primary constraint. Such a vignette would serve the purpose of allowing 'the function of the software [to] be verified' as well as providing some nice implicit documentation that maps the arguments of your sim function to the model results.

paul-buerkner commented 5 years ago

Thanks for reviewing this pacakge so thoroughly!

I must admit I am not overly enthusiastic about this request, as a proper simulation study either requires a cluster or months of patience. We have verified the parameter recovery extensively in one of our papers (https://psyarxiv.com/dbwn8/) but I of course get that this provides no tests going forward when changing the code base.

What I can do is run a single well behaved simulation trial with all three engines that takes a few hours at most and check parameter recovery on that basis. Would that be ok for you?

paul-buerkner commented 5 years ago

just pushed a vignette to github that contains a small simulation study to test parameter recovery as well as automated unit tests to both check the estimation accuracy and consistency across the different model fitting engines.

russellpierce commented 5 years ago

I haven't run the Vignette yet - but skimming the textual parts, this looks like exactly the right scope for the current problem; heck you went above and beyond here - thank you.