Docu test and review#7
Conversation
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Free Run ID: 📒 Files selected for processing (21)
💤 Files with no reviewable changes (5)
📝 WalkthroughWalkthroughThis PR refactors the dose-sensitivity workflow by consolidating utility functions directly into top-level R scripts with improved parameter configurability, and substantially expands documentation with comprehensive module specifications for drug PK/PD modeling and comprehensive TB immune response modeling within a MoBi QSP framework. ChangesDose Sensitivity and Scenario Workflow Refactoring
Module Documentation and Workflow Specification
🎯 4 (Complex) | ⏱️ ~60 minutes Note 🎁 Summarized by CodeRabbit FreeYour organization is on the Free plan. CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please upgrade your subscription to CodeRabbit Pro by visiting https://app.coderabbit.ai/login. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the TB modelling platform by introducing dynamic results and output folder management, cleaning up unused utility functions, and adding comprehensive mathematical model descriptions for the TB disease and drug PK/PD modules. The workflow documentation and README have also been updated to reflect these structural changes. The reviewer feedback highlights a critical bug in Code/utils/utils.R where simulation results are saved to a hardcoded directory instead of the dynamic resultsFolder. Additionally, the default settings in Code/workflowConfig.R are noted as fragile due to hardcoded scenario indices and commented-out drug configurations. Finally, several typos, copy-paste errors, and an undefined citation key were identified across the documentation and report files.
| resultsFolder <- file.path( | ||
| './Results/doseSensitivity/', | ||
| format(Sys.time(), "%Y-%m-%d %H-%M") | ||
| ) |
There was a problem hiding this comment.
While resultsFolder is correctly initialized here to a timestamped directory (or passed from the caller), the actual simulation results and CSV exports inside calculateEBA (when loadResults is FALSE) are hardcoded to ./Results/doseSensitivity/ (see lines 190 and 197 of Code/utils/utils.R). This means the files are saved in the parent directory rather than the specific resultsFolder, causing loadScenarioResults to fail when loadResults is TRUE because it looks inside resultsFolder.
| # Set scenariosToRun to NULL to run all (or all except scenariosNotToRun). | ||
| # scenariosToRun takes priority over scenariosNotToRun when both are set. | ||
| scenariosToRun <- NULL | ||
| scenariosToRun <- c(1:3) |
There was a problem hiding this comment.
Using a hardcoded numeric index range like c(1:3) as the default for scenariosToRun is fragile. If the user's ProjectConfiguration.xlsx contains fewer than 3 scenarios, names(scenarioConfigurations)[c(1:3)] will introduce NA values. This will propagate into selectedConfigurations as NULL elements, causing downstream functions like createScenarios or loadScenarioResults to crash. It is safer to default to NULL (to run all scenarios) or use explicit scenario names.
| dosePath = "Events|MOX PO|Tablet|Application_1|ProtocolSchemaItem|Dose", | ||
| doseUnit = "mg" | ||
| ) | ||
| ) #, |
There was a problem hiding this comment.
Most of the drug configurations in drugConfigs have been commented out, leaving only INHf active. While this might be useful for local testing, committing these commented-out blocks to the repository limits the default execution of the dose sensitivity analysis for other users. Consider keeping the full list active by default.
| - **EBA~0–5~**: covers the first five days of treatment. | ||
| - **EBA~0–14~**: integrates bactericidal activity over the full 14-day early phase. | ||
| - **EBA~2–5~**: reflects activity against the semi-dormant population that becomes prominent after the initial rapid kill. | ||
| Early Bactericidal Activity (EBA) is a key pharmacodynamic endpoint in tuberculosis (TB) drug development. It quantifies the rate of decline of viable *Mycobacterium tuberculosis* colony-forming units (CFU) in sputum during the early phase of monotherapy, expressed as the daily log~10~ reduction in CFU per mL. EBA measurements over different time windows capture distinct aspects of drug action, e.g. **EBA~0–2~** (rapid killiing (days 0–2)), **EBA~2–5~** (delayed effect), or **EBA~0–14~** (overall score/ more longterm effect). |
There was a problem hiding this comment.
There is a typo in this line: "killiing" should be "killing". Also, "longterm" is more commonly written as "long-term".
Early Bactericidal Activity (EBA) is a key pharmacodynamic endpoint in tuberculosis (TB) drug development. It quantifies the rate of decline of viable *Mycobacterium tuberculosis* colony-forming units (CFU) in sputum during the early phase of monotherapy, expressed as the daily log~10~ reduction in CFU per mL. EBA measurements over different time windows capture distinct aspects of drug action, e.g. **EBA~0–2~** (rapid killing (days 0–2)), **EBA~2–5~** (delayed effect), or **EBA~0–14~** (overall score/ more long-term effect).
| | A3 | Resistant bacteria carry a fixed fitness cost (`resistanceFitness` = 0.8 relative to wild type) | Biological | Compensatory mutations that restore fitness are not represented | | ||
| | A4 | Drug concentrations in Interstitial and Lesion compartments are provided exogenously via spatial parameters (`DRUG_compartment_e`, `DRUG_compartment_i`) | Pharmacokinetic | Tissue distribution kinetics must be supplied by a companion PBPK or effect-compartment module | | ||
| | A5 | Drug clearance from the central compartment follows first-order kinetics with linear induction over time up to `tmax` days | Pharmacokinetic | Non-linear autoinduction or saturable elimination are not captured in the simple PK model | | ||
| | A6 | Only mono-resistant strain are modelled | Pharmacological/Evolutionary |mulit-resistant strains are not captured although they are known to exist | |
There was a problem hiding this comment.
There are typos in this line: "strain" should be plural "strains" to match "are modelled", and "mulit-resistant" should be "multi-resistant".
| A6 | Only mono-resistant strains are modelled | Pharmacological/Evolutionary |multi-resistant strains are not captured although they are known to exist |
| immune killing. The net killing rate parameter `K_Be_total` is assembled in the | ||
| Spatial Structure and drives this reaction. | ||
|
|
||
| `K_Be_total` is defined as a sum over `K_Be_fast` or `K_Be_slow` of the implmented drugs depending on the growth phase of the bacteria. |
There was a problem hiding this comment.
There is a typo in this line: "implmented" should be "implemented".
`K_Be_total` is defined as a sum over `K_Be_fast` or `K_Be_slow` of the implemented drugs depending on the growth phase of the bacteria.
|
|
||
| The reaction contains the following local parameter definitions: |
There was a problem hiding this comment.
This sentence is a copy-paste error from the DRUG module documentation. The base TB Immune Scaffold module does not contain the drug-resistant species Be_r_DRUG and Bi_r_DRUG. This second sentence should be removed.
The macrophage passive transport building blocks are shared with the TB Immune Scaffold.
|
|
||
| `Code/utils/transformDrugParams.R` reads drug parameter text files from the | ||
| reference model implementation and converts them to the Excel format expected | ||
| reference model implementation @Fors and converts them to the Excel format expected |
There was a problem hiding this comment.
The citation key @Fors is not defined in references.bib. It should be @fors2020 to match the correct reference.
reference model implementation @fors2020 and converts them to the Excel format expected
1 participant
Summary by CodeRabbit
New Features
Documentation
Refactor