DOI-USGS / lake-temperature-process-models

Creative Commons Zero v1.0 Universal
1 stars 4 forks source link

Current passing of nmls as single object triggers unnecessary rebuilds w/ change to lake ids vector #22

Closed hcorson-dosch-usgs closed 2 years ago

hcorson-dosch-usgs commented 2 years ago

I noticed while working with a subset of p1_lake_cell_tile_xwalk_df to test code to package up the GLM output (#20, #21) that if I added a new site_id after having run the models for the existing subset of lakes, that all of the models rebuilt, which is definitely not the behavior we want. After digging a bit I realized it was due to an oversight in how I'm passing the nmls to run_glm3_model() when building p2_glm_uncalibrated_runs. Since we are mapping over p1_model_config to build p2_glm_uncalibrated_runs, which has 18 rows per lake, we couldn't also map over p1_nml_objects, which has 1 nested list element per lake. Therefore I passed in the full set of nml objects (p1_nml_objects) which gets updated each time the vector of site_ids changes. That's triggering the unnecessary rebuilds.

After talking it through, and some investigation of targets options on Lindsay's part, Lindsay and I decided that the best approach would be to save the nmls as files after we modify them, and then add the nml filepath to the model config when we build it, so that within run_glm_3_model() we can simply read in the nml file, instead of loading it from the list of nml objects. That way we won't need to pass in p1_nml_objects as an argument to build p2_glm_uncalibrated_runs.

lindsayplatt commented 2 years ago

@hcorson-dosch when you link code lines, try using the main repo and not your fork first (unless necessary!). Those links in the last paragraph are broken because that branch no longer exists on your fork!

hcorson-dosch-usgs commented 2 years ago

oh, shoot. thanks for the heads up! Will fix now