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

Error in cohortDefinitions[[i]]: subscript out of bounds #18

Closed
lhjohn opened this issue Nov 22, 2021 · 6 comments
Closed

Error in cohortDefinitions[[i]]: subscript out of bounds #18

lhjohn opened this issue Nov 22, 2021 · 6 comments

Comments

@lhjohn
Copy link
Collaborator

lhjohn commented Nov 22, 2021

When creating a validation package with the following parameters (skeletonVersion = "v1.0.4", saveModelsToJson = T) the following exception is thrown.

Error in cohortDefinitions[[i]]: subscript out of bounds

Interestingly, this error only occurs if there is only a single analysis. As soon as there are more analyses (e.g. by just duplicating Analysis_1) it works as intended.

@lhjohn
Copy link
Collaborator Author

lhjohn commented Nov 22, 2021

This error is generated in combineCohortDefinitions(), after the follow line.

cohortDefinitions <- lapply(1:length(modelsJson), function(i){modelsJson[[i]]$cohortDefinitions})

@jreps Could you have a look at this? I am not entirely following the logic. The way I understand it is that there are at least two cohort definitions in every study (target and outcome). But for some reason the length of cohortDefinitions becomes 1 after the line above. As a result this for-loop fails a few lines later.

for(i in 2:length(cohortDefinitions)){
  cohortDefinitionsAll <- append(cohortDefinitionsAll, cohortDefinitions[[i]])
}

@jreps
Copy link
Collaborator

jreps commented Dec 23, 2021

Can you send me the package you are trying to run so I can debug it?

@lhjohn
Copy link
Collaborator Author

lhjohn commented Dec 27, 2021

I forwarded it using SURFfilesender. Just run CodeToRun.R.

I left in some debug statements in combineCohortDefinitions() in the createValidationPackageJson.R file.

This uses v1.0.4 of the validation package, which is in Hydra and the problem occurs when there is only a single Analysis present, for multiple analyses it seems to work.

Ultimately, we probably want to use the latest SkeletonPredictionValidationStudy automatically instead of the Hydra version, but I also had problems with SkeletonPredictionValidationStudy Issue#12

@jreps
Copy link
Collaborator

jreps commented Jan 6, 2022

I should have just fixed the issue. Is this the only issue you had?

I'm getting a Hydra error as the skeleton is in develop rather than master - if you use the develop version of Hydra instead of the renv version it should build. I'm writing code in the latest skeleton to optionally skip Hydra though.

@lhjohn
Copy link
Collaborator Author

lhjohn commented Jan 6, 2022

This was the only issue I encountered with SkeletonPredictionStudy. But as you mentioned, it would be great to use SkeletonPredictionValidationStudy directly and skip Hydra when creating validation packages.

But in SkeletonPredictionValidationStudy I am still seeing Issue#12.

Maybe we could work on moving some of the things in develop to master in both packages? I gladly test them using some of my latest studies.

@jreps
Copy link
Collaborator

jreps commented Jan 6, 2022

After restructuring PLP the skeletons are being updated (which is why I have to use renv for testing the old packages). It would be best to start using the issue 242 version of the skeletons as these work with the new PLP and we can test these, fix any issues and then make these the master version.

In the issue 242 branch I've currently updated the model development/protocol for the development skeleton but am still working on the code to create the validation package (using Hydra or skipping Hydra)

@jreps jreps closed this as completed Mar 24, 2022
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

No branches or pull requests

2 participants