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

enhancement for mxStandardizeRAMpaths: ParamsCov[paramnames, paramnames] : subscript out of bounds #49

Closed
tbates opened this issue Feb 7, 2018 · 11 comments
Assignees
Labels
Milestone

Comments

@tbates
Copy link
Member

tbates commented Feb 7, 2018

Load up the attached RAM model (a supermodel with 2 submodels)

load("https://github.com/OpenMx/OpenMx/files/1704191/eqmeans.rda.txt")

running mxStandardizeRAMpaths yields the following error:

mxStandardizeRAMpaths(eqmeans@submodels[[2]], SE = TRUE)
Error in ParamsCov[paramnames, paramnames] : subscript out of bounds

It works fine if SE is not requested:

mxStandardizeRAMpaths(eqmeans@submodels[[2]], SE = FALSE)
        name             label matrix  row  col  Raw.Value        Raw.SE  Std.Value    Std.SE
 1 DZ.S[1,1] bmi1_with_bmi1_DZ      S bmi1 bmi1 0.88451032 not requested 1.00      not requested
 2 DZ.S[2,1] bmi1_with_bmi2_DZ      S bmi2 bmi1 0.46340859 not requested 0.52      not requested
 3 DZ.S[2,2] bmi2_with_bmi2_DZ      S bmi2 bmi2 0.88395528 not requested 1.00      not requested

Also works fine on the other submodel

mxStandardizeRAMpaths(eqmeans@submodels[[2]], SE = TRUE)
       name             label matrix  row  col  Raw.Value      Raw.SE  Std.Value                     Std.SE
1 MZ.S[1,1] bmi1_with_bmi1_MZ      S bmi1 bmi1 0.88238409 0.020542628 1.000    3.9984e-13
2 MZ.S[2,1] bmi1_with_bmi2_MZ      S bmi2 bmi1 0.46613570 0.016449657 0.528   1.1986e-02
3 MZ.S[2,2] bmi2_with_bmi2_MZ      S bmi2 bmi2 0.88174717 0.020505953 1.000   1.5254e-15

OpenMx version: 2.8.3.71 [GIT v2.8.3-71-g0aa1fc0]
R version: R version 3.4.3 (2017-11-30)
Platform: x86_64-apple-darwin15.6.0
MacOS: 10.13.4
Default optimiser: NPSOL

eqmeans.rda.txt

@tbates tbates added the bug label Feb 7, 2018
@tbates tbates added this to the Boulder 2018 milestone Feb 7, 2018
@RMKirkpatrick
Copy link
Contributor

I'd be curious to see the syntax that produced eqmeans. I observe:

> names(omxGetParameters(eqmeans$DZ))
[1] "bmi1_with_bmi1_DZ" "bmi1_with_bmi2_DZ" "bmi2_with_bmi2_DZ" "one_to_bmi_DZ"
> rownames(eqmeans$DZ$output$hessian)
[1] "bmi1_with_bmi1_DZ" "bmi1_with_bmi2_DZ" "bmi2_with_bmi2_DZ" "one_to_bmi1_DZ"    "one_to_bmi2_DZ"

The DZ submodel's Hessian has rows and columns corresponding to nonexistent free parameters, whereas free parameter one_to_bmi_DZ is not represented in the Hessian. How on earth did you manage to do that?! And why do the submodels have their own Hessians in the first place? Dependent submodels in a multigroup analysis shouldn't have their own Hessians. I suspect something weird happened well before the call to mxStandardizeRAMpaths().

@RMKirkpatrick
Copy link
Contributor

OK, I see that the container model, as well as both submodels, have their own compute plans. That would explain why the submodels have their own Hessians.

@tbates
Copy link
Member Author

tbates commented Feb 7, 2018

The code is just paths and models. The models have different data, different path labels, different parameter estimates. Not sure how Hessian could be the same, or how a user might even attempt to do that, or to apply a single compute plan. I just build models, and created a supermodel with a group objective. No advanced edits of any kind were made.

Re stale names in Hessian, It might be that the path names are updated but the model hasn't been re-run. But in that case the bug would be mxStandardizeRAMpaths not checking a model has been run? Or isn't dirty?

But we also shouldn't allow raw errors out of functions like ParamsCov (not sure what that is)
Maybe add a check to ensure all paramnames are present whereever they are being looked for and report if they aren't? "subscript out of bounds" doesn't help.

@tbates
Copy link
Member Author

tbates commented Feb 7, 2018

Some more information: offer up the container model to mxStandardizeRAMpaths, and it returns a list with both submodel's standardized paths as expected:

mxStandardizeRAMpaths(eqmeans, SE=T)
$MZ
       name             label matrix  row  col Raw.Value     Raw.SE Std.Value       Std.SE
1 MZ.S[1,1] bmi1_with_bmi1_MZ      S bmi1 bmi1 0.8823846 0.02054263 1.0000000 7.273597e-14
2 MZ.S[2,1] bmi1_with_bmi2_MZ      S bmi2 bmi1 0.4661361 0.01644971 0.5284595 1.198671e-02
3 MZ.S[2,2] bmi2_with_bmi2_MZ      S bmi2 bmi2 0.8817476 0.02050591 1.0000000 5.125615e-14

$DZ
       name             label matrix  row  col Raw.Value     Raw.SE Std.Value       Std.SE
1 DZ.S[1,1] bmi1_with_bmi1_DZ      S bmi1 bmi1 0.8845109 0.02063196   1.00000 2.502178e-13
2 DZ.S[2,1] bmi1_with_bmi2_DZ      S bmi2 bmi1 0.4634090 0.01646017   0.52408 1.206146e-02
3 DZ.S[2,2] bmi2_with_bmi2_DZ      S bmi2 bmi2 0.8839560 0.02059778   1.00000 3.220089e-13

But ask for the DZ model, and the error emerges:

mxStandardizeRAMpaths(eqmeans$DZ, SE=T)
Error in ParamsCov[paramnames, paramnames] : subscript out of bounds

While for the MZ model, it works fine

mxStandardizeRAMpaths(eqmeans$MZ, SE=T)
       name             label matrix  row  col Raw.Value     Raw.SE Std.Value       Std.SE
1 MZ.S[1,1] bmi1_with_bmi1_MZ      S bmi1 bmi1 0.8823846 0.02054263 1.0000000 7.273598e-14
2 MZ.S[2,1] bmi1_with_bmi2_MZ      S bmi2 bmi1 0.4661361 0.01644966 0.5284595 1.198670e-02
3 MZ.S[2,2] bmi2_with_bmi2_MZ      S bmi2 bmi2 0.8817476 0.02050595 1.0000000 5.125626e-14

@mcneale
Copy link
Contributor

mcneale commented Feb 8, 2018 via email

@RMKirkpatrick
Copy link
Contributor

Joshua says that the backend only looks at the compute plan in the container MxModel, so the fact that the submodels have their own Hessians isn't a result of them having their own compute plans. Did you run the MZ and DZ models separately, and then put them together into a multigroup container?

Some more information: offer up the container model to mxStandardizeRAMpaths, and it returns a list with both submodel's standardized paths as expected:

Yes, here mxStandardizeRAMpaths() is working as designed: it's being applied to the container MxModel in a multigroup analysis. The container model's Hessian has entries for all of the free parameters.

But in that case the bug would be mxStandardizeRAMpaths not checking a model has been run? Or isn't dirty?

mxStandardizeRAMpaths() does check to see if the model has been run, and if it hasn't, coerces SE to FALSE (with a warning). It doesn't check to see if the model has been modified since it was run. It probably should, but that would not make a difference in this case: eqmeans$DZ@.modifiedSinceRun is FALSE.

Maybe add a check to ensure all paramnames are present whereever they are being looked for and report if they aren't?

Perhaps. Again, I'm going to need to see the syntax that gave rise to eqmeans. eqmeans presents a scenario I did not think was even possible (so of course I didn't write mxStandardizeRAMpaths() to accommodate it). I need to know how it came into being before I can decide on what the correct bug-fix would be.

@tbates
Copy link
Member Author

tbates commented Feb 8, 2018

Ahh: yes, models each run on their own first, as the supermodel struggled if the submodels didn't already have good starting solutions.

Sounds like the thing that needs doing is to cope with the fact that models might have Hessians lying around from being run independently? Something like omx_not_run_as_top_level_model ?

Or maybe trap this kind of parameter mismatch error with:

"I don't appear to have been run on my own for a while - perhaps you want to use umxStandardizeRAM on my container model instead of me?"

I think the issue was that I didn't know umxStandardizeRAM could cope with submodels, so maybe this won't bite anyone but me trying to write helper functions.

@tbates tbates changed the title bug in mxStandardizeRAMpaths: ParamsCov[paramnames, paramnames] : subscript out of bounds enhancement for mxStandardizeRAMpaths: ParamsCov[paramnames, paramnames] : subscript out of bounds Feb 8, 2018
@RMKirkpatrick
Copy link
Contributor

RMKirkpatrick commented Feb 8, 2018

Ahh: yes, models each run on their own first, as the supermodel struggled if the submodels didn't already have good starting solutions.

So, then the mismatch between free parameter labels and Hessian dimnames in the DZ submodel arose because you ran the assembled container model through omxSetParameters(), and (I'm guessing) changed two different parameter labels to the same new label?

In any event, running mxStandardizeRAMpaths() on eqmeans$DZ constitutes a corner case that is unlikely to occur in ordinary use, but we now know that it is nonetheless possible. There should probably be a check for all the elements of paramnames being dimnames in ParamsCov, and an error (with a helpful message) raised if the check fails. And, mxStandardizeRAMpaths() should check the .modifiedSinceRun slot.

Edit: I was wrong about it not checking the .modifiedSinceRun slot. It does, by a call to assertModelFreshlyRun().

@RMKirkpatrick
Copy link
Contributor

Should be repaired with 25eab2c, though it still needs a test.

@tbates
Copy link
Member Author

tbates commented Mar 6, 2018

Closing as now has a better error message

Under OpenMx version 2.9.4 I get:

load("~/Downloads/eqmeans.rda", verbose = TRUE)
mxStandardizeRAMpaths(eqmeans@submodels[[2]], SE = TRUE)
Error in .mxStandardizeRAMhelper(model = model, SE = SE, ParamsCov = covParam) : 
  some free parameter labels do not appear in the dimnames of the parameter estimates' covariance matrix;
are you running mxStandardizeRAMpaths() on a dependent submodel instead of on its multigroup container model?
the missing parameter labels are:
one_to_bmi_DZ
In addition: Warning message:
In warnModelCreatedByOldVersion(model) :
  You are using OpenMx version 2.9.4 with a model created by OpenMx version 2.8.3.71. This may work fine (fingers crossed), but if you run into trouble then please recreate your model with the current version of OpenMx.

@tbates tbates closed this as completed Mar 6, 2018
@RMKirkpatrick
Copy link
Contributor

@tbates Could you add a test for that error message? The only reason I left this open was the lack of a test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants