Format model inputs in p2a_model.R

[lekoenig]

This PR addresses part of #159, namely that there were redundant COMID columns in the inputs_and_outputs data frame in p2a_well_obs (e.g. COMID.x, COMID.y). Here I've added COMID as a join key when building p2a_well_obs to omit the redundant columns.

I've also added some comments and made formatting edits in both 2a_model.R and 2a_model/src/model_ready_data_utils.R to 1) add documentation that will allow us/me to better track the different steps in 2a_model.R; and 2) format some function documentation to match the code style used throughout the rest of the repo.

[galengorski]

Hi @lekoenig this looks good to me! I'm wondering if there is a way that we can map the snakemake dependencies per your comments on lines 171-174, I always find it confusing to track down which of the snakefile need changing, but maybe that is just the nature of a complex pipeline.

[lekoenig]

I'm wondering if there is a way that we can map the snakemake dependencies per your comments on lines 171-174, I always find it confusing to track down which of the snakefile need changing, but maybe that is just the nature of a complex pipeline.

I agree and would love to incorporate any suggestions for taking the burden of tracking dependencies out of our heads and into comments and code! Do you have an example of what you mean by mapping the snakemake dependencies? Are you referring to added documentation somewhere?

[lekoenig]

My thought (hope?) was to treat the _targets.R file as a sort of one-stop-shop location for configuration options that relate to the data + model pipeline. Hopefully this would reduce the cognitive burden of knowing where to go to update data or modeling options (even if you're not familiar with the individual data processing steps in the targets pipeline or all of the modeling steps in the snakemake pipeline).

So now we include the model configuration options in _targets.R and the configuration yaml files get built by the targets pipeline. And I think that individual lines in the .smk file should never need to be manually changed, but I'm less familiar with those files and could be wrong about that.

I suggest we think about this further and make additional changes when we tackle #146. I'll go ahead and merge this PR for now.