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

Add PatientLevelPredictionValidationModule & ModelTransferModule #161

Draft
wants to merge 5 commits into
base: v1.0-main
Choose a base branch
from

Conversation

anthonysena
Copy link
Collaborator

@anthonysena anthonysena commented Sep 3, 2024

Aims to migrate the PatientLevelPredictionValidationModule & ModelTransferModule to the new Strategus v1.0 format.

This PR is lacking test cases at the moment which I plan to add with some guidance from @egillax.

targetId = df$target_id[1],
outcomeId = df$outcome_id[1],
plpModelList = as.list(df$modelPath),
restrictPlpDataSettings = jobContext$settings[[1]]$restrictPlpDataSettings,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Leaving this here for @egillax - just wanted to note that the module's settings implies a list of lists for the validationComponentsList but here we're only using the first element of the list?

Copy link

Choose a reason for hiding this comment

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

Yeah that's not correct. Thanks for catching that. I as well see some other issues, the createPatientLevelPredictionValidationModuleSpecifications input shouldn't have a validationSettings item in the list of lists and I should as well document what the function expects as input.

Would you accept a PR to this branch fixing those things ?

# )

# check the model locations are valid and apply model
upperWorkDir <- dirname(jobContext$moduleExecutionSettings$workFolder) # AGS: NOTE - Using the "root" folder as the expection is that the ModelTransferModule output is here
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Linking this to #21 since I'm unsure of the workflow between the ModelTransferModule and the PLP Validation module. In looking at the upperWorkDir the assumption I see is that the ModelTransferModule is written to the "root" folder that holds the results/work sub folders. Is this correct?

Copy link

Choose a reason for hiding this comment

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

I'm not sure I completely follow. The ModelTransferModule should write the models to either results/work module subfolder in a directory models. The current code writes it in the work subfolder. Then it writes a csv file in the results directory that has the paths to each individual model in the work directory.

So something like:

├── strategusOutput
│   ├── DatabaseMetaData
|   ├── CohortGeneratorModule
│   ├── DatabaseMetaData
│   ├── ModelTransferModule
│   │   └── github_export.csv
├── strategusWork
│   ├── CohortGeneratorModule
│   ├── ModelTransferModule
│   │   └── models
│   │       └── model_github_PandemicPrediction_master
│   │           ├── model_1
│   │           ├── model_2

Since the downloaded models are the results of the ModelTransferModule it could be argued they should be in the results subfolder instead (strategusOutput/ModelTransferModule/models) . But I would defer to you which fits better in with Strategus.

In current code the upperWorkDir should be the strategusWork directory. And in line 61 modelSavelocation should point to strategusWork/ModelTransferModule/models and that's where the validation module looks for the models. I'm using sort(dir(upperWorkDir, pattern = 'ModelTransferModule'), decreasing = T)[1] to identify the correct module since it used to have trailing underscores and numbers. If that's not the case anymore this can be simplified to a simple file.path.

Hope that's clear!
Egill

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