-
Notifications
You must be signed in to change notification settings - Fork 6
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
feat: allow exposure_workflow to read input config yml + refactor for compatibility with converted_util files as input #99
base: main
Are you sure you want to change the base?
Conversation
…l + update func params for yml cfg
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.
Looking great! Added a few comments ...
data-raw/input_config.yml
Outdated
future_climate: "./future_climate_data_new.rds" | ||
species: "./species_new.rds" | ||
exposure_result_file: "exposure_result.rds" | ||
workers: 1 |
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.
can you add a comment that omitting the workers
parameter will result in availableCores()-1
workers being used? Or perhaps that should the default and we could just include the line commented out? E.g.
# workers: 1 # This parameter can be used to use a specific number of workers instead of the system default
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.
updated
docker_exposure.sh
Outdated
@@ -2,11 +2,13 @@ | |||
source run_container.sh | |||
|
|||
if [ $# -lt 2 ]; then | |||
echo "usage: $0 <data_dir> <output_dir> <extra-args>" | |||
echo "usage: $0 <input_config_yml_file_path> <output_dir> <extra-args>" |
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.
We no longer use extra-args, right? Since they come through the yaml?
echo "usage: $0 <input_config_yml_file_path> <output_dir> <extra-args>" | |
echo "usage: $0 <input_config_yml_file_path> <output_dir>" |
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.
updated.
scripts/exposure_workflow.R
Outdated
# Load all functions from the package | ||
log_info("Loading package functions...") | ||
devtools::load_all() | ||
log_info("Package functions loaded successfully.") | ||
|
||
# 1. Transform the distribution polygons to match the grid | ||
log_info("Transforming distribution polygons to match the grid.") | ||
primates_range_data <- prepare_range(primates_shp, grid) | ||
log_info("Processed {length(primates_range_data)} species.") |
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 suggest changing the wording of these log entries to something like "loaded" vs "Processed" or "Extraction done" since we are no longer doing any processing here.
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.
updated.
scripts/main.R
Outdated
@@ -197,32 +217,76 @@ run_climatearray2rds <- function(args) { | |||
} | |||
|
|||
run_exposure <- function(args) { | |||
|
|||
if (length(args) < 2) { |
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.
This should be taken care of by safe_parse_opts()
which will provide a better error.
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.
updated - now as a part of check_not_null()
#88 #98
exposure
command line to read input_config.yml. Example: Rscript scripts/main.R exposure -i input_config.ymlexposure_time_workflow()
exposure_time_workflow()
that is already done by converted_util functions.Execution via docker:
sh docker_exposure.sh "./data-raw/input_config.yml" "./outputs"
Execution via CL locally:
Rscript scripts/main.R exposure -i data-raw/input_config.yml