-
Notifications
You must be signed in to change notification settings - Fork 8
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
Fims concurrent #520
Fims concurrent #520
Conversation
417ff34
to
7afa9a7
Compare
Instructions for code reviewerHello reviewer, thanks for taking the time to review this PR!
Checklist
|
Codecov ReportAll modified and coverable lines are covered by tests ✅ see 13 files with indirect coverage changes 📢 Thoughts on this report? Let us know! |
PR could use a description under "What is the feature?" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with Ian, including a feature description would be beneficial.
This pull request aims to integrate several scripts for comparing the performance of running FIMS using {Rmpi}, {processR}, and {snowfall}. There are a couple of options for incorporating these files into the codebase:
- Convert the scripts into a test using the testthat framework.
- Draft a vignette that compares the performance of running FIMS with {Rmpi}, {processR}, and {snowfall}.
However, the feature branch needs to be merged and closed this week before working on issue #312 that removes files from the FIMS repository history. I suggest we create a new issue outlining the options and remaining tasks before merging these files.
What is the feature?
How have you implemented the solution?
Does the PR impact any other area of the project?
How to test this change
Developer pre-PR checklist