UW-Hydro / VIC

The Variable Infiltration Capacity (VIC) Macroscale Hydrologic Model
http://vic.readthedocs.io
MIT License
259 stars 385 forks source link

Eliminated unnecessary unit conversions for resid_moist etc. #850

Closed tbohn closed 5 years ago

tbohn commented 5 years ago

Currently, the soil_con terms max_moist, Wcr, and Wpwp, have units of [mm] but resid_moist has units of [mm/mm]. Every time resid_moist is used in the model physics, it is first multiplied by depth * MM_PER_M for consistency with soil moisture units. Not only is this needless extra computation, but it also makes debugging and modification of the code needlessly difficult. Furthermore, it requires adding depth as an argument to various functions for the sole purpose of appearing in the unit conversion. There are also a few instances in which other soil moisture thresholds have their units converted needlessly (either other variables with the desired units already exist, or the units actually get converted BACK again before use!).

This PR does the following:

dgergel commented 5 years ago

@tbohn can you post a few figures from when you tested this branch so that we can confirm that results are identical with these changes?

dgergel commented 5 years ago

@tbohn thanks. Once you've posted some plots then this will be good to go.

tbohn commented 5 years ago

Differences in results between this branch and the current develop branch are 0.

  1. classic driver: image
  2. image driver: image image
tbohn commented 5 years ago

junk

This is the output of gridcell [1,1] in the Stehekin dataset. "Develop" = current develop branch, and "SoilParamUnits" is this PR.

dgergel commented 5 years ago

@tbohn thanks, looks good to me.