Open alyst opened 6 months ago
I think it's very nice to unify this, we only have to decide which names to land on.
identifier
, we should definitely get rid of it. I personally prefer full words over abbreviations, so I would go for parameter
, parameters()
and nparameters()
. I also think this is more in line with general Julia style, reading the docs "avoid abbreviation ... as it becomes difficult to remember whether and how particular words are abbreviated.".variables
to vars
and covariance
to cov
, because I believe everyone will understand these, and everything else is written out (meaning we have to change obs_cov
to observed_cov
, but I thinks thats actually nice because it may not be obvious immediately that obs
refers to observed
).
n_man
is very confusing indeed^^observed
in the context of missing
to avoid confusion.parameter_type
to relation
?@aaronpeikert @brandmaier I think it's important to have these things in line with how the community usually refers to them - so maybe you can also have a look. Andreas, do you like the missing
/ measured
naming or would you use something different?
See also #158 #159.
I quite like the suggestions. What do you think about the idea to deal with abbreviations by consistently declaring both abbreviation and fully spelled out version, e.g.:
const cov = covariance
I don't like to use observed/latent (because of e.g. observed_cov
), and would prefer manifest/latent.
consistently declaring both abbreviation and fully spelled out version, e.g.: const cov = covariance
cov
exists in StatsBase.jl. I personally don't think there's a need to be more explicit than standard Julia packages.
Aliases may create confusion -- which one you use in the examples, which one you actually use in the package code etc.
For the users it also may not be 100% obvious which is alias of which -- one has to look up it in the docs, but then it is just as easy as to document the short version right away.
I don't like to use observed/latent (because of e.g. observed_cov), and would prefer manifest/latent.
How do you define observed_cov
? Is it the covariations based on the classical formula using only the observed data matrix without missing values?
SemObservedMissing
uses multivariate normal EM to calculate the implicit covariations between the manifested variables.
AFAIU these covariations are not exposed through public API right now, but I think it makes perfect sense to expose them, be it observed_cov
or manifested_cov
.
Actually, in my staging branch I have modified SemObservedMissing
, so it could be used in SemML
just the same way as SemObservedData
(SemML
relies that the data object implements SemData
interface); and it provides the EM-based covariations via observed_cov
.
And, potentially, there could be other methods for estimating the covariations of the observed (manifested) variables.
So I think it makes sense to have a generic SemData
method that provides these covariations (and another method for the means).
One solution is to have both observed_cov()
and manifested_cov()
-- but I expect that observed_cov()
will have very limited usage both internally and externally.
I agree cov is pretty clear. Just saying if we really want to use an abbreviation we could define an alias for the long form but of course we should always use the short form. But we dont have to, just a thought.
How do you define observed_cov? Is it the covariations based on the classical formula using only the observed data matrix without missing values?
Yes. For me it is about disambiguation of model implied covariance, and "observed" in the sense that it is what the data provide. So that we have manifest/latent and observed/implied. However, your point about how we deal with missing data using the implied covariance of the unrestricted model as the "observed" cov fuzzies this line.
I'm also not so happy with the aliases because I think it will create too much confusion...
I think "observed covariance matrix" is kind of the standard SEM textbook lingo in normal ML estimation (without missings), so I generally prefer observed_cov
over manifest_cov
, but I would be happy with both. I believe whether we call the EM-based covariance observed_cov
should be decided when you do the PR with those changes, because I would have to look at the code first to see how SemData
etc. is implemented.
As a part of #193 I already made some changes, so I wanted to get the feedback from maintainers about it. Plus, there are a few other changes in the same direction that I can integrate into #193, so I wanted to mention them here too.
param
(intuitively understandable, but still short):param
in the ParTableparams()
to get the vector of parametersnparams()
to get the number of parameters (calledn_par()
now)vars()
to get the vector of variables fromParTable
,RAMMatrices
(matching the order ofA
columns)nvars()
to get the number of variablesobserved_vars()
to get the observed variables matching the order of rows/cols inobs_cov
and rows ofRAMMatrices.F
Alternatively, it could beobs_vars()
, which would matchobs_cov()
andobs_mean()
(ifobserved_vars
is chosen, thenobs_cov
also needs be renamed intoobserved_cov
for consistency).nobserved_vars()
to get the number of observed vars (replacesn_man
, which in this short form is a little bit confusing).latent_var_indices()
/observed_var_indices()
to get the indices ofvars()
that match the observed/latent variables (i-th index ofobserved_var_indices()
is for the i-th variable ofobserved_vars()
)latent_vars()
is a shortcut tovars()[latent_var_indices()]
samples
to access to the individual samples (sometimes referred to asrows
orrowwise
).nsamples()
is the number of samples (n_obs()
now)<-
or<->
). Now theParTable
have the inparam_type
column, which is confusing, because sometimes it is constant.