Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider removing LOG_MATRIC option #65

Closed
tbohn opened this issue Jan 1, 2014 · 6 comments
Closed

Consider removing LOG_MATRIC option #65

tbohn opened this issue Jan 1, 2014 · 6 comments
Assignees
Labels
Milestone

Comments

@tbohn
Copy link
Contributor

tbohn commented Jan 1, 2014

Preliminary tests indicate that the LOG_MATRIC run-time option (formerly the LOW_RES_MOIST compile-time option in user_def.h) has little effect.

The preliminary tests were conducted at a few grid cells in the Columbia Basin. It would be nice to conduct further tests over a wider variety of environments to confirm whether this option is necessary.

If it is deemed unnecessary, its removal should be relatively straightforward, as it primarily affects runoff.c.

@ghost ghost assigned tbohn Jan 3, 2014
@jhamman
Copy link
Member

jhamman commented Sep 25, 2014

@tbohn or @bartnijssen , do either of you have any thoughts thoughts on this. The documentation for the option is sparse but this is what I've been able to find.

If TRUE VIC uses the linear interpolation of the logarithm of the
matric potential from the two surrounding layers to estimate the
soil moisture drainage from each layer (Boone and Wetzel, 1996).
This should improve the soil moisture drainage predicted by the
low resolution solution computed by VIC.

The option is used twice in runoff.c

343       if (options.LOG_MATRIC) {
344         for( lindex = 0; lindex < options.Nlayer; lindex++ ) {
345           if( (tmp_liq = liq[lindex] - evap[lindex][frost_area]) < resid_moist[lindex] )
346             tmp_liq = resid_moist[lindex];
347           if(tmp_liq > resid_moist[lindex])
348             matric[lindex] = soil_con->bubble[lindex] * pow( (tmp_liq - resid_moist[lindex]) / (soil_con->max_moist[l    348 index] - resid_moist[lindex]), -b[lindex]);
349           else
350           matric[lindex] = HUGE_RESIST;
351         }
352       }

...and...

366           if (options.LOG_MATRIC) {
367             avg_matric = pow( 10, (soil_con->depth[lindex+1] * log10(fabs(matric[lindex])) + soil_con->depth[lindex]     367 * log10(fabs(matric[lindex+1]))) / (soil_con->depth[lindex] + soil_con->depth[lindex+1]) );
368             tmp_liq = resid_moist[lindex] + ( soil_con->max_moist[lindex] - resid_moist[lindex] ) * pow( ( avg_matric    368  / soil_con->bubble[lindex] ), -1/b[lindex] );
369           }

I see three options moving forward with this issue:

  1. Leave the option as is implemented now but provide documentation on the global parameter webpage.
  2. The LOG_MATRIC=FALSE option: Remove the option and the LOG_MATRIC code.
  3. The LOG_MATRIC=True option: Remove the option and make the LOG_MATRIC code the default treatment of soil moisture drainage.

@tbohn
Copy link
Contributor Author

tbohn commented Sep 25, 2014

The way I see it, it was an experimental option that was never fully tested
(implemented before my time). The few tests I performed on it several
months ago indicated that it didn't make a big difference. But those
weren't comprehensive tests. If it turns out to make a difference to the
results, then replacing the current default ("false") with this ("true")
would change everyone's calibrations, so it seems to me it would need to be
proven to be a huge improvement for option 3 ("true") to be worth it. If
it turns out to NOT make a difference, there's no point in keeping it.

Meanwhile, option 1 requires more effort on our parts as well, for
something that probably few if any people use.

So I'm leaning towards option 2 (removing it).

Ted

On Thu, Sep 25, 2014 at 8:21 AM, Joe Hamman notifications@github.com
wrote:

@tbohn https://github.com/tbohn or @bartnijssen
https://github.com/bartnijssen , do either of you have any thoughts
thoughts on this. The documentation for the option is sparse but this is
what I've been able to find.

If TRUE VIC uses the linear interpolation of the logarithm of the
matric potential from the two surrounding layers to estimate the
soil moisture drainage from each layer (Boone and Wetzel, 1996).
This should improve the soil moisture drainage predicted by the
low resolution solution computed by VIC.

The option is used twice in runoff.c
https://github.com/UW-Hydro/VIC/blob/develop/src/runoff.c#L343-L352

343 if (options.LOG_MATRIC) {344 for( lindex = 0; lindex < options.Nlayer; lindex++ ) {345 if( (tmp_liq = liq[lindex] - evap[lindex][frost_area]) < resid_moist[lindex] )346 tmp_liq = resid_moist[lindex];347 if(tmp_liq > resid_moist[lindex])348 matric[lindex] = soil_con->bubble[lindex] * pow( (tmp_liq - resid_moist[lindex]) / (soil_con->max_moist[l 348 index] - resid_moist[lindex]), -b[lindex]);349 else350 matric[lindex] = HUGE_RESIST;351 }352 }

...and...

366 if (options.LOG_MATRIC) {367 avg_matric = pow( 10, (soil_con->depth[lindex+1] * log10(fabs(matric[lindex])) + soil_con->depth[lindex] 367 * log10(fabs(matric[lindex+1]))) / (soil_con->depth[lindex] + soil_con->depth[lindex+1]) );368 tmp_liq = resid_moist[lindex] + ( soil_con->max_moist[lindex] - resid_moist[lindex] ) * pow( ( avg_matric 368 / soil_con->bubble[lindex] ), -
<
span class="mi">1/b[lindex] );369 }370 Q12[lindex] = Ksat[lindex] * pow(((tmp_liq - resid_moist[lindex]) / (soil_con->max_moist[lindex] - resid_mo 370 ist[lindex])), soil_con->
expt[lindex]); 371 }372 else Q12[lindex] = 0.;373 last_layer[last_cnt] = lindex;374 }

I see three options moving forward with this issue:

  1. Leave the option as is implemented now but provide documentation on
    the global parameter webpage
    http://www.hydro.washington.edu/Lettenmaier/Models/VIC/Documentation/GlobalParam.shtml
    .
  2. The LOG_MATRIC=FALSE option: Remove the option and the LOG_MATRIC
    code.
    1. The LOG_MATRIC=True option: Remove the option and make the
      LOG_MATRIC code the default treatment of soil moisture drainage.


Reply to this email directly or view it on GitHub
#65 (comment).

@jhamman
Copy link
Member

jhamman commented Sep 25, 2014

The above PR (#143) is not set in stone but does illustrate how easy it is to remove the option entirely.

@bartnijssen
Copy link
Member

Removed by merging PR #143

@jhamman
Copy link
Member

jhamman commented Dec 5, 2014

I'm reopening since I'm seeing a few pieces of the LOG_MATRIC option still in the code.

@jhamman jhamman reopened this Dec 5, 2014
@jhamman
Copy link
Member

jhamman commented Dec 14, 2014

close again via 1b7d23a

@jhamman jhamman closed this as completed Dec 14, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants