USGS-R / drb-gw-hw-model-prep

Code repo to prepare groundwater and headwater-related datasets for modeling river temperature in the Delaware River Basin
Other
0 stars 3 forks source link

Refactor targets pipeline phase names #57

Closed lekoenig closed 1 year ago

lekoenig commented 1 year ago

This PR replaces the 2_process phase with two more descriptive phases, 2a_process_nhd_downscaling and 2b_process_nhm_groundwater. No targets or functions have been changed, but nevertheless the code changes here represent a considerable refactor of the targets pipeline (note that 1_fetch.R is left unchanged for now).

The rationale for making this change was that 2_process.R was pretty bulky (consisting of 28 targets) and it was hard to navigate between the two "sub-projects" or "model experiments" that are represented in this pipeline. Most of the targets previously in 2_process.R belong cleanly within the set of targets related to the NHD downscaling experiments (13 targets now in 2a_process_nhd_downscaling.R) or the set of targets related to compiling groundwater-relevant attributes (15 targets now in 2b_process_nhm_groundwater.R). The following targets get used in both phases: p2a_static_inputs_nhd_formatted and p2a_static_inputs_prms.

I think this will be helpful as we flesh out methods text and documentation related to each of these two model experiments, and will hopefully be more intuitive for future collaborators (e.g. probably us 6 months from now 😉). What do you think?

Unfortunately these changes did result in all the "p2" targets re-building for me. It took a few hours to re-run the full pipeline.

Closes #23

msleckman commented 1 year ago

Looks good! I like the split. The pipeline runs for me with this update.

Let's aim to update the readme with a quick descriptor that describes the 3 pipeline scripts and explains the reasoning for the split in 2_process

lekoenig commented 1 year ago

Let's aim to update the readme with a quick descriptor that describes the 3 pipeline scripts and explains the reasoning for the split in 2_process

Great idea! I've created a new issue to flesh out the pipeline README.