-
Notifications
You must be signed in to change notification settings - Fork 15
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
added fit by group table #145
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks great, just a couple of comments
@@ -203,7 +203,7 @@ checkLavaanModel <- function(model, availableVars) { | |||
modelContainer <- createJaspContainer() | |||
modelContainer$dependOn(c("samplingWeights", "meanStructure", "manifestInterceptFixedToZero", "latentInterceptFixedToZero", "exogenousCovariateFixed", "orthogonal", | |||
"factorScaling", "residualSingleIndicatorOmitted", "residualVariance", "exogenousLatentCorrelation", | |||
"dependentCorrelation", "threshold", "scalingParameter", "efaConstrained", "standardizedVariable", "naAction", "estimator", "test", | |||
"dependentCorrelation", "threshold", "scalingParameter", "efaConstrained", "standardizedVariable", "naAction", "estimator", "modelTest", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we change it here? Was this a bug?
@@ -360,11 +360,11 @@ checkLavaanModel <- function(model, availableVars) { | |||
lavopts[["estimator"]] <- options[["estimator"]] | |||
lavopts[["se"]] <- ifelse(options[["errorCalculationMethod"]] == "bootstrap", "standard", options[["errorCalculationMethod"]]) | |||
lavopts[["information"]] <- options[["informationMatrix"]] | |||
lavopts[["test"]] <- ifelse(options[["modelTest"]] == "satorraBentler", "Satorra.Bentler", | |||
ifelse(options[["modelTest"]] == "yuanBentler", "Yuan.Bentler", | |||
lavopts[["test"]] <- ifelse(options[["modelTest"]] == "satorraBentler", "satorra.bentler", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The same here, was this a bug?
R/sem.R
Outdated
grouptab$addColumnInfo(name = "AIC", title = gettext("AIC"), type = "number" ) | ||
grouptab$addColumnInfo(name = "BIC", title = gettext("BIC"), type = "number" ) | ||
grouptab$addColumnInfo(name = "N", title = gettext("n"), type = "integer") | ||
grouptab$addColumnInfo(name = "Chisq", title = gettext("χ²"), type = "number" , |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
grouptab$addColumnInfo(name = "Chisq", title = gettext("χ²"), type = "number" , | |
grouptab$addColumnInfo(name = "Chisq", title = "\u03C7\u00B2", type = "number" , |
R/sem.R
Outdated
grouptab$addColumnInfo(name = "PrChisq", title = gettext("p"), type = "pvalue", | ||
overtitle = gettext("Baseline test")) | ||
if (length(options[["models"]]) > 1) { | ||
grouptab$addColumnInfo(name = "dchisq", title = gettext("\u0394\u03C7\u00B2"), type = "number" , |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
grouptab$addColumnInfo(name = "dchisq", title = gettext("\u0394\u03C7\u00B2"), type = "number" , | |
grouptab$addColumnInfo(name = "dchisq", title = "\u0394\u03C7\u00B2", type = "number" , |
if it's only symbols we don't need to translate
R/sem.R
Outdated
if (length(options[["models"]]) > 1) { | ||
grouptab$addColumnInfo(name = "dchisq", title = gettext("\u0394\u03C7\u00B2"), type = "number" , | ||
overtitle = gettext("Difference test")) | ||
grouptab$addColumnInfo(name = "ddf", title = gettext("\u0394df"), type = "integer", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
here we only want to translate the "df"
grouptab$addColumnInfo(name = "ddf", title = gettext("\u0394df"), type = "integer", | |
grouptab$addColumnInfo(name = "ddf", title = gettextf("%1$sdf", "\u0394"), type = "integer", |
6daf3fe
to
9ad5223
Compare
9ad5223
to
a1a2d04
Compare
a1a2d04
to
f10573d
Compare
although this seems wrong to me, why would the same model with fewer Ns have fewer Df? |
It also seems very annoying that we have to fit the model again for each group, if only lavaan would give us the AIC and BIC |
R/sem.R
Outdated
equalityConstraints <- syntax[syntax$op == "==",] | ||
if (nrow(equalityConstraints) > 0) { | ||
for (j in 1:nrow(equalityConstraints)) { | ||
if(equalityConstraints[j, "lhs"] %in% syntax_group[, "plabel"]) { | ||
syntax_group <- rbind(syntax_group, equalityConstraints[j,]) | ||
} | ||
} | ||
} | ||
syntax_group[["group"]] <- syntax_group[["block"]] <- rep(1, nrow(syntax_group)) | ||
lav_args[["model"]] <- syntax_group | ||
dataset_group <- dataset[dataset[[options[["group"]]]] == groupNames[group],] | ||
lav_args[["data"]] <- dataset_group | ||
fit_group <- try(do.call(lavaan::lavaan, lav_args)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Kucharssim, this code does not work when there are equality constraints, which is not surprising. Lorenzo fits the model for the subgroup only, but with invariance testing equality constraints are between the two groups, e.g., the loadings of group1 are equal to the loadings of group2. It seems to me that the equality constraints do not matter for the fit of each group, so we should estimate the model with free parameters unless some the constraints are not between groups but within a group. I wonder how we would find distinguish here though....
6b9c520
to
a6c1b7d
Compare
0531f85
to
9a90250
Compare
ce79b5d
to
5332b4e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The functionality looks really good, I didn't find anything suspicious in the code...
The tests fail spectacularly though, can you look into that? It seems to me these failures could be introduced by the PR, so it would be good to polish that before merging
5332b4e
to
f7c732e
Compare
I suppose I should've checked, weird that I didn't notice the test failing. It's caused by imposing meanstructure but then not fixing any intercepts to 0. I wonder if in qml there is an option for something like a triple of checkboxes, they can be all checked but not unchecked simultaneously. Suppose .quitAnalysis will have to do for now. |
b09b7c3
to
6fda55c
Compare
6fda55c
to
588e4e5
Compare
jasp-stats/jasp-issues#711 (fixed for SEM)
fixes jasp-stats/jasp-issues#2690
also adds chi^2 diff tests for multiple models and groups