Feature/clean up esoil #777

tbohn · 2018-02-03T00:59:48Z

Description:

Problem: the logic in computing soil evaporation (esoil) is not very clean nor does it make clear which portion of ET comes from esoil, until the partitioning is cleared up in put_data().
Solution: (a) introduce explicit esoil, transp, and evap terms into the layer struct; (b) remove the bare_evap_frac term; (c) modify logic in func_surf_energy_bal() and other functions to use these terms; and (d) make the logic cleaner

jhamman · 2018-02-13T08:52:12Z

This looks good to me. Let's sort out #779 before we merge this though.

dgergel · 2018-02-20T05:04:43Z

vic/vic_run/src/canopy_evap.c

-       evaporation is taken from the wetter layer.  Otherwise layers
-       contribute to evapotransipration based on root fraction.
+       transpiration is taken from the wetter layer.  Otherwise layers
+       contribute to transipration based on root fraction.


transipration --> transpiration

OK, will fix.

dgergel · 2018-02-20T05:06:13Z

vic/vic_run/src/canopy_evap.c

@@ -494,32 +495,32 @@ transpiration(layer_data_struct *layer,
    }

    /****************************************************************
-       Check that evapotransipration does not cause soil moisture to
+       Check that transipration does not cause soil moisture to


transipration --> transpiration

OK, will fix.

dgergel · 2018-02-20T05:07:36Z

vic/vic_run/src/func_surf_energy_bal.c

-                transp[i] = layer[i].evap;
-                layer[i].evap = 0.;
-            }
+    for (i=0; i<options.Nlayer; i++) {


did you already run uncrustify? this line has some formatting issues

I'll fix it by hand. Unfortunately, when I run uncrustify on my system, using the provided config file, it makes a ton of unwanted changes in files that I've never touched. Not sure why.

dgergel · 2018-02-20T05:12:12Z

vic/vic_run/src/func_surf_energy_bal.c

+    Evap = 0.;
+    if (!SNOWING) {
+        if (VEG && veg_var->fcanopy > 0) {
+            Evap = canopy_evap(layer, veg_var, true, veg_class, Wdew,


I'm not taking issue with this, but I don't understand the motivation behind separating out the (!SNOWING) from the VEG and veg_var->fcanopy into separate lines?

I minimized redundant code. If you start with a condition that links the 3 conditions VEG, !SNOWING, and fcanopy > 0, you'll necessarily need further compound conditions later on to handle all possible cases. The most important case is simply !SNOWING. If it is snowing, Evap is 0. So, I separated that from the other conditions. Then, the nested check on VEG and fcanopy > 0 determines whether we call canopy_evap() and how SurfRad is set. Note: I'm now removing lines 782-784, since they were redundant with my initialization of Evap to 0 on line 751.

Got it - thanks for clarifying

dgergel · 2018-02-20T05:13:31Z

vic/vic_run/src/func_surf_energy_bal.c

-                veg_var->throughfall;
-            veg_var->canopyevap *= veg_var->fcanopy;
-            veg_var->Wdew *= veg_var->fcanopy;
+            SurfRad = surf_atten*NetBareRad;


more formatting issues

Will fix by hand (see previous formatting comment).

dgergel · 2018-02-20T05:14:48Z

This looks good to me, just a few small comments and a question. Also it looks like uncrustify needs to be run before this is merged, there are some formatting issues

…e/clean_up_esoil

tbohn · 2018-02-20T05:35:58Z

Just to reiterate, I can't run uncrustify because for some reason it doesn't behave the same on my machine as it does on yours at UW. It changes lots of files that I haven't touched, and formats them in a way that I'm pretty sure you won't like. I don't know why; I'm using the accompanying config file. But until I get that fixed, my workaround is to do the formatting by hand.

dgergel · 2018-02-20T05:55:22Z

I run uncrustify on my laptop locally - what version are you using @tbohn? My experience is that whenever I run uncrustify it often reformats things that are not visible to the (my) naked eye - so I think it is worth figuring out why it isn't working for you rather than manually fixing things. That's my two cents at least

tbohn · 2018-02-20T06:01:39Z

Understood.

tbohn · 2018-02-22T20:21:10Z

OK, fixed the uncrustify issue on my machine, and have run uncrustify on the code.

dgergel · 2018-02-23T07:32:03Z

vic/drivers/shared_all/src/put_data.c

+        outflow,
+        storage,
+        save_data->
+        total_moist_storage);


this reformatting looks off to me

Well, the original exceeded the 80-character limit, as far as I can tell, so it looks to me like the reformatting did the right thing in that respect... How exactly should it look?

I could change it by hand but then uncrustify might attempt to change it again in future PRs...

dgergel · 2018-02-23T07:35:24Z

@tbohn looks good to me, I just had one small comment where the formatting looks funky

…en reformatted by uncrustify in a weird way.

tbohn · 2018-02-23T16:25:54Z

I went ahead and changed the formatting by hand anyway.

dgergel · 2018-02-26T18:36:45Z

I think this all looks good. @jhamman?

ted added 10 commits December 31, 2017 13:04

Cleaned up esoil logic and changed default value of RARC_SOIL to 250.

d2aefec

Merge branch 'develop' into feature/clean_up_esoil

c0e4d6c

Rolled back the change to SOIL_RARC.

2e89b81

Merge branch 'feature/clean_up_vic_run' into feature/clean_up_esoil

e79328f

Added release notes entry.

4aa7df4

Merge branch 'feature/clean_up_vic_run' into feature/clean_up_esoil

08b0645

Merge branch 'feature/clean_up_vic_run' into feature/clean_up_esoil

6f4bf10

Merge branch 'feature/clean_up_vic_run' into feature/clean_up_esoil

4e8706c

Merge branch 'feature/clean_up_vic_run' into feature/clean_up_esoil

db1f67c

Merge branch 'develop' into feature/clean_up_esoil

d071912

tbohn force-pushed the feature/clean_up_esoil branch from 845cca0 to d071912 Compare February 3, 2018 16:52

jhamman requested a review from dgergel February 13, 2018 08:52

dgergel reviewed Feb 20, 2018

View reviewed changes

ted added 2 commits February 19, 2018 22:28

Minor edits in response to code review.

23af624

Merge branch 'develop' of https://github.com/UW-Hydro/VIC into featur…

edf426f

…e/clean_up_esoil

tbohn force-pushed the feature/clean_up_esoil branch from d864718 to edf426f Compare February 20, 2018 05:31

ted added 2 commits February 21, 2018 11:11

Merge branch 'develop' into feature/clean_up_esoil

c3fb00e

Ran uncrustify.

8b038bf

dgergel reviewed Feb 23, 2018

View reviewed changes

Changed formatting of call to calc_water_balance_error(), that had be…

c406c48

…en reformatted by uncrustify in a weird way.

ted and others added 2 commits April 24, 2018 10:36

Merge branch 'develop' into feature/clean_up_esoil

aa29a2a

Merge branch 'develop' into feature/clean_up_esoil

0e70e1e

jhamman merged commit 0aed9a1 into UW-Hydro:develop Apr 25, 2018

tbohn mentioned this pull request Sep 28, 2018

Cleanup: make ET partitioning more explicit #765

Closed

tbohn deleted the feature/clean_up_esoil branch September 28, 2018 22:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/clean up esoil #777

Feature/clean up esoil #777

tbohn commented Feb 3, 2018 •

edited

Loading

jhamman commented Feb 13, 2018

dgergel Feb 20, 2018

tbohn Feb 20, 2018

dgergel Feb 20, 2018

tbohn Feb 20, 2018

dgergel Feb 20, 2018

tbohn Feb 20, 2018

dgergel Feb 20, 2018

tbohn Feb 20, 2018

dgergel Feb 20, 2018

dgergel Feb 20, 2018

tbohn Feb 20, 2018

dgergel commented Feb 20, 2018 •

edited

Loading

tbohn commented Feb 20, 2018

dgergel commented Feb 20, 2018

tbohn commented Feb 20, 2018

tbohn commented Feb 22, 2018

dgergel Feb 23, 2018

tbohn Feb 23, 2018

dgergel commented Feb 23, 2018

tbohn commented Feb 23, 2018

dgergel commented Feb 26, 2018

Feature/clean up esoil #777

Feature/clean up esoil #777

Conversation

tbohn commented Feb 3, 2018 • edited Loading

jhamman commented Feb 13, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dgergel commented Feb 20, 2018 • edited Loading

tbohn commented Feb 20, 2018

dgergel commented Feb 20, 2018

tbohn commented Feb 20, 2018

tbohn commented Feb 22, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dgergel commented Feb 23, 2018

tbohn commented Feb 23, 2018

dgergel commented Feb 26, 2018

tbohn commented Feb 3, 2018 •

edited

Loading

dgergel commented Feb 20, 2018 •

edited

Loading