Skip to content
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

Check dimension conformability of all arguments #230

Open
mhunter1 opened this issue Aug 8, 2019 · 6 comments
Open

Check dimension conformability of all arguments #230

mhunter1 opened this issue Aug 8, 2019 · 6 comments
Labels
better errors Make better error and/or warning messages critical task

Comments

@mhunter1
Copy link
Owner

mhunter1 commented Aug 8, 2019

Original report by Michael Hunter (Bitbucket: mhunter, GitHub: mhunter).


A user created a seg fault by having 1 observed variable but a 4x4 measurement noise covariance.

@mhunter1 mhunter1 added this to the Coding Session 1 milestone Jan 14, 2020
@mhunter1 mhunter1 added the better errors Make better error and/or warning messages label Jan 14, 2020
@mhunter1
Copy link
Owner Author

mhunter1 commented Feb 4, 2020

Here's the conformability check function from OpenMx

checkSSMConformable <- function(mat, rows, cols, matname, modname){
	if( nrow(mat) != rows || ncol(mat) != cols ){
		msg <- paste("The ", matname, " matrix is not the correct size",
			" in the state space expectation of model ", modname,
			".  It is ", nrow(mat), " by ", ncol(mat), " and should be ",
			rows, " by ", cols, ".", sep="")
		stop(msg, call. = FALSE)
	}
}

@mhunter1
Copy link
Owner Author

mhunter1 commented Feb 4, 2020

Here's how the matrices get checked.

		# Conformability checking
		ldim <- ncol(Cmatrix)
		mdim <- nrow(Cmatrix)
		checkSSMConformable(Amatrix, ldim, ldim, 'A', omxQuotes(modelname))
		checkSSMConformable(Cmatrix, mdim, ldim, 'C', omxQuotes(modelname))
		checkSSMConformable(Qmatrix, ldim, ldim, 'Q', omxQuotes(modelname))
		checkSSMConformable(Rmatrix, mdim, mdim, 'R', omxQuotes(modelname))
		checkSSMConformable(Xmatrix, ldim, 1, 'x0', omxQuotes(modelname))
		checkSSMConformable(Pmatrix, ldim, ldim, 'P0', omxQuotes(modelname))

@symiin
Copy link
Collaborator

symiin commented Feb 4, 2020

In dynr.model, check conformability of the followings:

  1. dynamics (Meng and Hui-Ju)
    -Check the length of the list of formulas. Compare length to the length of list in prep.initials.
  • Within each set of formulas, compare number of formulas against length of state.names from measurement model.
    -Check length of startvals to see if it matches the length of parameters after name checking
  1. noise (Yanling and Jonathan)
  • Check dimension of measurement noise covariance matrix to see if it matches length of obs.names
    -Check dimension of process noise covariance matrix against number of formulas and also length of state.names from prep.measurement.
  1. initial (Mike and Jungmin)
    -Check number of rows in initial condition mean vector to see if it matches the length of state.names
    -Check dimension of initial condition covariance matrix to see if the dimension matches the length of state.names
    -In initial: Check the length of the initial regime probabilities vector to see if it matches the length of the list of elements in measurement; in dynamics

  2. regimes (Linying and Sy-Miin)

  • Check the dimension of the regime transition logodds matrix to see if it is the right dimension with and without covariates
  • Check the dimension against the lengths of lists of elements in measurement and dynamics

@mhunter1
Copy link
Owner Author

mhunter1 commented Feb 11, 2020

Maybe this is a trivial distinction, but I recommend a slightly different strategy. Instead of checking everything against everything else, use the strategy outlined below.

Here's how the matrices get checked.

		# Conformability checking
		ldim <- ncol(Cmatrix)
		mdim <- nrow(Cmatrix)
		checkSSMConformable(Amatrix, ldim, ldim, 'A', omxQuotes(modelname))
		checkSSMConformable(Cmatrix, mdim, ldim, 'C', omxQuotes(modelname))
		checkSSMConformable(Qmatrix, ldim, ldim, 'Q', omxQuotes(modelname))
		checkSSMConformable(Rmatrix, mdim, mdim, 'R', omxQuotes(modelname))
		checkSSMConformable(Xmatrix, ldim, 1, 'x0', omxQuotes(modelname))
		checkSSMConformable(Pmatrix, ldim, ldim, 'P0', omxQuotes(modelname))

That is, set some ground truth for

  1. the number of regimes
  2. the number of latent variables
  3. the number of observed variables
  4. the names of the latent variables
  5. the names of the observed variables

Then check that everything that should match this ground truth does in fact match it.

As evidenced by the code I wrote for OpenMx, I like the idea of having the measurement model (e.g. prep.measurement) define the number and names for both latent and observed variables.

Set ground truths

  • Set number of regimes nr
  • Set number of latent variables ne
  • Set names of latent variables nne
  • Set number of observed variables ny
  • Set names of observed variables nny
  • Set the names and numbers of covariates

Dynamics (@meng-chen-2013 @hjhung)

  • Check that prep.formulaDynamics formulas have ne left-hand sides
  • Check that prep.formulaDynamics formulas use all and only nne in left-hand sides
  • Check that prep.matrixDynamics is ne by ne

Noise (@yanlingli1 @JPark93)

  • Check that measurement noise is ny by ny
  • Check that dynamic noise is ne by ne

Initial (@mhunter1 @ares7823)

  • Check that ini.state is ne by 1
  • Check that ini.cov is ne by ne
  • Check that regimep is nr by 1

Regimes (@symiin linying)

  • Check that the dimension of the regime transition logodds matrix is the right dimension with and without covariates
  • Check that nr does not contradict the number of regimes in dynamics, noise, or initial

Repository owner deleted a comment from symiin Feb 11, 2020
@mchdfs
Copy link
Collaborator

mchdfs commented Feb 25, 2020

I put in an additional check for the class of dynamic argument in dynr.model:

if(!class(dynamics) %in% c("dynrDynamicsFormula","dynrDynamicsMatrix")){
stop("Check to see that dynamics argument is of the correct class. Hint: it should be either 'dynrDynamicsFormula' or 'dynrDynamicsMatrix'.")
}

in case people put in a set of formulae instead.

@mhunter1
Copy link
Owner Author

@meng-chen-2013 This is a nice idea. I think we should go all the way with it. See #248.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
better errors Make better error and/or warning messages critical task
Projects
None yet
Development

No branches or pull requests

3 participants