Address Review Comments#40
Merged
Merged
Conversation
… Also include link to GNU General Public License (future LICENSE file).
…sating and compiling 'pbpk_simple.model' seems to require so much time that it causes devtools::build() to time out.
…anuscript Merging from central repository.
Closed
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR implements several review action items for the MCSimMod package, primarily focused on updating documentation and adding new demonstration content. The changes enhance the package's educational value and clarify how the software works with ODE models.
Key changes include:
- Addition of new vignettes demonstrating package functionality and PBPK modeling
- Updates to existing vignette numbering and cross-references for better organization
- Addition of comprehensive documentation for paper submission including figures and bibliography
Reviewed Changes
Copilot reviewed 24 out of 57 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| vignettes/details.Rmd | New vignette explaining MCSimMod details and object-oriented design |
| vignettes/pbpk_demo.Rmd | New comprehensive PBPK model demonstration vignette |
| vignettes/model_specification.Rmd | Enhanced model specification vignette with detailed element descriptions |
| inst/extdata/pbpk_simple.model | New PBPK model specification file for demonstrations |
| paper/paper.md | Complete paper submission with summary and statement of need |
| LICENSE | Addition of GPL-3 license file |
| R/compileModel.R | Enhanced error handling and logging messages |
| DESCRIPTION | Version update and dependency reorganization |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Action items to address:
stringi,DT, andhtmltoolsare packages that are required for installation. [We believe that all packages on which MCSimMod depends are correctly listed in the DESCRIPTION file. No errors are produced in our continuous integration (CI) checks for installation of MCSimMod on Unix and Windows systems.]createModel()andcompileModel(). [The new vignette 'details.Rmd' provides descriptions ofcreateModel()andcompileModel().]ssdtoolspackage (https://joss.theoj.org/papers/10.21105/joss.07492), which I previously reviewed, offers a good example." [We have added a statement to the paper: "Examples of model specification files and vignettes that demonstrate how to use them to perform simulations are included in the MCSimMod package." We also added two new vignettes, "details.Rmd" and "pbpk_demo.Rmd" that provide information about how to use the MCSimMod system for working with ODE models.]mrgsolve." [While we agree that analyzing and comparing computational efficiency would be interesting and worthwhile, it is beyond the scope of this paper.]