Skip to content

Comments

Cleanup fitmeasures mg#311

Open
Maximilian-Stefan-Ernst wants to merge 10 commits intodevelfrom
cleanup_fitmeasures_mg
Open

Cleanup fitmeasures mg#311
Maximilian-Stefan-Ernst wants to merge 10 commits intodevelfrom
cleanup_fitmeasures_mg

Conversation

@Maximilian-Stefan-Ernst
Copy link
Collaborator

@Maximilian-Stefan-Ernst Maximilian-Stefan-Ernst commented Feb 19, 2026

This PR introduces the following changes:

Fit measures:

  • stricter checks for computing fit measures to ensure they are only returned when we know it is appropriate to compute them
    • single models are only allowed to have one loss function
    • multigroup models need to have one single loss function in each group, that is the same across groups
  • the RMSEA is now based on the corrected number of samples (following ML and GLS estimation)

Multigroup models:

  • for parameter estimation, WLS multigroup weights are now (N_i-1)/(N-G) instead of N
  • the chi^2 fit statistic is now based on the sum of the group chi^2

Comment on lines +22 to 23
χ²(::SemWLS, fit::SemFit, model::AbstractSemSingle) =
(nsamples(fit) - 1) * fit.minimum

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[JuliaFormatter] reported by reviewdog 🐶

Suggested change
χ²(::SemWLS, fit::SemFit, model::AbstractSemSingle) =
(nsamples(fit) - 1) * fit.minimum
χ²(::SemWLS, fit::SemFit, model::AbstractSemSingle) = (nsamples(fit) - 1) * fit.minimum

Comment on lines +234 to +235
Default weights of (#samples per group/#total samples) will be used".
return [(nsamples(model)) / (nsamples_total) for model in models]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[JuliaFormatter] reported by reviewdog 🐶

Suggested change
Default weights of (#samples per group/#total samples) will be used".
return [(nsamples(model)) / (nsamples_total) for model in models]
Default weights of (#samples per group/#total samples) will be used".return [
(nsamples(model)) / (nsamples_total) for model in models
]

Comment on lines +240 to +241
Default weights of (#samples per group/#total samples) will be used".
return [(nsamples(model)) / (nsamples_total) for model in models]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[JuliaFormatter] reported by reviewdog 🐶

Suggested change
Default weights of (#samples per group/#total samples) will be used".
return [(nsamples(model)) / (nsamples_total) for model in models]
Default weights of (#samples per group/#total samples) will be used".return [
(nsamples(model)) / (nsamples_total) for model in models
]

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@alyst
Copy link
Contributor

alyst commented Feb 19, 2026

@Maximilian-Stefan-Ernst I think these are the nice improvements! Just to let you know that the last remaining big piece in #193 is refactoring around SEM Loss functions and ensembles, which unifies SEM models and SEM ensembles (eliminates SemEnsemble type, because from optimization perspective it should not matter if multiple terms are SEM models or different priors/regularizations) and makes it a bit easier to construct multi-group models, have multiple different regularization terms and control their weights. It will be no problem for me to rebase these changes on top of the changes you have here -- given, of course, that you would consider these refactorings are worth merging. I mention it here because handling of single models / ensembles in fit measures might be a bit more streamlined after the refactoring.

@Maximilian-Stefan-Ernst
Copy link
Collaborator Author

That sounds super cool - I would definitely consider these changes. If you have any comments to this PR now, I would also be happy for a review.

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.

2 participants