ecmerkle / blavaan

An R package for Bayesian structural equation modeling
https://ecmerkle.github.io/blavaan
87 stars 23 forks source link

Added moment match option for fitIndices #46

Closed lstmemery closed 2 years ago

lstmemery commented 2 years ago

Hey there, I have the PR for this issue https://github.com/ecmerkle/blavaan/issues/43.

I have the feature change and some tests, but I'm not sure how to add documentation of the feature. Do I just edit the Rd files manually? Also, I seem to only get p_loo reporting, even when I explicitly ask for looic. Any idea when that happens?

Lastly, I run a tiny stan model twice instead of saving it as test data. I was having difficulty properly saving the model into sysdata.rda. If you think that slows down the tests too much, could you add it? Note that I can't NULL the stanmodel, because it's used by the rstan::loo function.

ecmerkle commented 2 years ago

Thanks! I will look at the looic issue and everything else soon. About sysdata.rda: I'm not sure that it is a big deal, because the stan estimation is currently pretty fast (sysdata was more useful for older estimation methods that took longer). I suspect that what you did will be fine.

Finally, about Rd files: yes, it is manual editing. Roxygen is one of those things I often think about but never get around to.

lstmemery commented 2 years ago

Ok, sounds good! I'll update the Rd files and try to figure out why the CMD CHECK is failing (It worked locally, I swear!).

By the way, I saw while updating my fork that you had a commit saying that FitIndices doesn't work on ordinal models. Is there more information about that? Is that something I can help with?

By the way, converting from raw Rd to Roxygen would probably be a good first issue for a newer programmer. I could make an issue and point some people to it, if you like. :)

Update: I just saw Issue #45.

ecmerkle commented 2 years ago

About ordinal fit indices: I have a couple more things to try there, to try to distinguish between "more research is needed" vs "bug in code".

About Roxygen: I would be open to help, though I guess that the Rd2roxygen package could do much of the work. If there is something requiring more work, it will probably be how to modify my current build scripts. I am most worried about the automated builds/checks that use github actions, because I don't fully understand how those work.

lstmemery commented 2 years ago

@ecmerkle Updated the documentation and the check should pass now. I had accidentally deleted sysdata.rda in the merge.

TDJorgensen commented 2 years ago

converting from raw Rd to Roxygen would probably be a good first issue for a newer programmer

Agreed. I used rd2roxygen for semTools when I started using roxygen2 for documentation. It works pretty well, but I eventually manually edited my source files to make it more readable (e.g., vertical spacing between longer sections of comments, like @params vs. @references vs. @examples).

ecmerkle commented 2 years ago

thanks for the contribution!