Description - Split MTCLIM out from VIC into a stand-alone executable. Remove OUTPUT_FORCE option.
Optional? - No
Comment - Sandalone MTCLIM would need to output in standard VIC forcing format (netCDF).

See issue #23 for more details on forcing format.

See later

Anyone have any idea what the file wgtpar.h does in the VIC source code directory? It seems to be a stray. A grep shows the file is not _include_d by any file and all the content seems to be hardcoded.

If it really is extraneous, it should be removed (we may want to make sure that there are no other unused files in the source tree). Since it's now under version control we can always revive it.

Description - Either remove these options or move them to global parameter file. Remove #ifdef’s from code. Remove VIC’s –o flag.
Optional? - No

Description - Accounts for heat flux between lake and underlying soil
Optional? - No, but not executed when no lake is present

No effect when the lake model is not implemented

Description - Photosynthesis, soil respiration
Optional? - Yes
Comment - Requires additional veg library parameters

Description - Accounts for wetland microtopography
Optional? - Yes
Needs modification:

• Replace hard-coded distribution with input from a parameter file
• Move more code into subroutines

Description - Ability to read vegetation parameters for each vegetation type from file at a daily time step
Optional - yes

Description - Couple RVIC to image mode drivers
Optional? - Yes

See issue #7 for more details regarding the image mode drivers.

The explicit scheme (currently the default) is not accurate for certain combinations of time step and node spacing. Currently VIC gives an error message if the user attempts to run the explicit scheme with these combinations. Given that the implicit scheme (currently optional) has no such restrictions and does not require substantially more cpu time than the explicit scheme, it would seem beneficial to remove the explicit scheme.

To allow for short model time steps and to standardize the time unit, make the basic time unit in VIC a second (rather than an hour as is currently. For example, run-times less than an hour are a must in most coupled applications.

Xu Liang (see below for contact info) has done some work on this

Model crashes in solve_T_profile under certain conditions when running with FROZEN_SOIL = TRUE and QUICK_FLUX = FALSE; currently handled with the Tfallback hack, but we would prefer a more robust, physically sound solution

We currently have a small sample dataset posted on the vic website for people to download and play with as an example. One reason for adding this to the archive could simply be because when people download the model source code, it's nice for them to just get the sample dataset with it.

Another reason would be because different VIC versions require different parameters and/or different formats for the input files. It would be good to be able to tag these and associate them with the version of the model that they correspond to.

Theoretically, this could be extended to archiving the inputs (and even outputs) of various projects. But I'm leaning away from that - it could take up a lot of space and wouldn't really lend itself to incremental deltas. It's probably best just to publish them elsewhere and make a note of the exact VIC version that worked with them. Perhaps the way to handle this is: if you archive a version of VIC (like my dissertation work) on a support branch, you should archive a small set of sample inputs (e.g., veg library, global parameter file with the various options visible, one cell's worth of soil, veg, etc. parameters and forcings and states) along with it?

Currently the logic accounting for a) iteration between surface and canopy temperature solutions, b) snow step vs model step, and c) distributed precip results in a confusing mess of data structures and loops. This makes adding features and preventing bugs more difficult than otherwise.

Description - Linear interpolation of monthly LAI
Optional? - Yes

Bug discovered in response to email to vic_users (https://mailman2.u.washington.edu/mailman/private/vic_users/2014-February/000410.html)

Memory for the snow band parameters is allocated in read_soilparam.c:

    temp.AreaFract     = (double *)calloc(Nbands,sizeof(double));
    temp.BandElev      = (float *)calloc(Nbands,sizeof(float));
    temp.Tfactor       = (double *)calloc(Nbands,sizeof(double));
    temp.Pfactor       = (double *)calloc(Nbands,sizeof(double));
    temp.AboveTreeLine = (char *)calloc(Nbands,sizeof(char));

When ARC_SOIL is set to TRUE, this code is not executed, because the equivalent code is not in read_soilparam_arc.c (nor anywhere else).

The model then produces a memory error in:

initialize_atmos:min_Tfactor = Tfactor[0]; 

if SNOW_BAND is set equal to 1 in the global file, or

read_snowband.c:soil_con->AreaFract[band]  = area_fract;

if SNOW_BAND is greater than 1 in the global file

Suggested solutions:

1. port the requisite code from read_soilparam.c to read_soilparam_arc.c. This would also require testing.
2. disable the ARC_SOIL option. In this case we should remove the PILPS 2e test data from the VIC downloads page (http://www.hydro.washington.edu/Lettenmaier/Models/VIC/SourceCode/Download.shtml).

My preference would be latter (since it involves less work). One option would be to provide a little utility to create the regular VIC soil file from the ARC files, so that we can still use the PILPS 2e test data and no one is left stranded.

Description - Use netCDF library to support binary I/O.
Optional? - No
Comment - Includes: state files, history files, parameter files, and forcings.

Hi Joe,

I believe this hotfix is ready for release. I had asked you to wait on it last week, in light of Homero's finding that melt_energy was getting garbage values intermittently (well, a single occurrence in a single grid cell, out of several grid cells). We couldn't find any concrete evidence that this problem was caused by any of the fixes in the hotfix.

Just as an aside: we ran VIC in valgrind to check for memory issues that could possibly cause garbage to pop up in unrelated variables. VIC came out clean.

Since Homero has opened a separate issue regarding melt_energy, it would seem that melt_energy can be dealt with in a separate hotfix. Of course it is your and Bart's call.

Ted

Description - Remove all loops over dist and dist array dimensions from code.
Optional? - No

While the COMPUTE_TREELINE and JULY_TAVG_SUPPLIED global parameter options and the July_Tavg soil parameter are described in the online documentation, it is not sufficiently clear to a new user that the July average air temperature needs to be supplied if their simulation does not contain any dates in July.

This might make a difference if VIC ever were to consider different lapse rates for Tmin and Tmax (which would result in different levels of shortwave radiation at different elevations).

After examining the snow melt energy output I concluded that the output values for that variable are not snow melting energy since they are non-zero mostly during the summer when SWE is zero. I don't know what that variable is outputing but since it's not affecting the rest of the variables I'm looking into, I won't look into it to move forward with my work. But I want others to know. Among things to check in the development of the model we should look into what variables are being used and which ones are relics and are not working properly and need to be repaired or eliminated.

Keith Cherkauer and Alan Hamlet have both done some work on this.

VIC starts on the wrong forcing record if not starting the simulation on hour 0. This is caused by an incomplete check in make_dmy.c. Make_dmy.c computes how many forcing records to skip (global->forceskip) by looking for the year and day of year specified as the start date. Start hour is ignored, so that the first forcing record of the day is selected as the starting point, even if the restart occurs mid-way through the day.

The fix is simply to add another condition to the if statement, so that the forcing record selected is the one corresponding to STARTHOUR.

I’d like to propose a standard for determining release names for VIC based on the Semantic Versioning 2.0.0 standard. The standard proposes a version format of X.Y.Z (Major.Minor.Patch). According to the standard:

Bug fixes not affecting the API increment the patch version, backwards compatible API additions/changes increment the minor version, and backwards incompatible API changes increment the major version

I’ll suggest that the VIC API can be sufficiently defined by the existing user interface. For this reason, major releases would be invoked when there is a change in the required inputs. As the VIC code becomes more modular in VIC 5.0, the defined API should be more specifically defined.

Lastly, I should acknowledge that this is essentially what has been done in the past; now with a clear explanation. The major changes are to lose the 4th digit (i.e. VIC.4.1.2.i) and to select the release name based on the contents of it's changes. This would have some impact on what we include in VIC.4.2.0. Changes such as #21 and #22 would need to wait until VIC.5.0.0.

Thoughts?

This would be very useful for climate forcings that have a "no_leap" calendar. LEAPYR is defined at the top of make_dmy.c.

Description - Reorganize to allow multiple patches having different soil, land cover, topography, etc. Regroup related elements together in more intuitive way. Requires data structure overhaul.
Optional? - No
Comment - For semi-backward compatibility, would require tools for parameter file conversion.

When ALMA_INPUT is TRUE, and forcing time step is not hourly, any moisture fluxes in the forcings (e.g. precipitation) will be scaled incorrectly in initialize_atmos.c. The fix is simply to add a check on options.ALMA_INPUT in the if statement surrounding the scaling.

I am unable to install GitHub for Windows 7. When I try to install, a window pops out saying,

"Unable to retrieve application files. Files corrupt in deployment."

It then shows the following details in a notepad.

PLATFORM VERSION INFO
Windows : 6.1.7600.0 (Win32NT)
Common Language Runtime : 4.0.30319.1
System.Deployment.dll : 4.0.30319.1 (RTMRel.030319-0100)
clr.dll : 4.0.30319.1 (RTMRel.030319-0100)
dfdll.dll : 4.0.30319.1 (RTMRel.030319-0100)
dfshim.dll : 4.0.31106.0 (Main.031106-0000)

SOURCES
Deployment url : http://github-windows.s3.amazonaws.com/GitHub.application
Server : AmazonS3
Application url : http://github-windows.s3.amazonaws.com/Application%20Files/GitHub_1_2_4_0/GitHub.exe.manifest
Server : AmazonS3

IDENTITIES
Deployment Identity : GitHub.application, Version=1.2.4.0, Culture=neutral, PublicKeyToken=317444273a93ac29, processorArchitecture=x86
Application Identity : GitHub.exe, Version=1.2.4.0, Culture=neutral, PublicKeyToken=317444273a93ac29, processorArchitecture=x86, type=win32

APPLICATION SUMMARY

• Installable application.
• Trust url parameter is set.
  ERROR SUMMARY
  Below is a summary of the errors, details of these errors are listed later in the log.
• Activation of http://github-windows.s3.amazonaws.com/GitHub.application resulted in exception. Following failure messages were detected:
• Exception occurred loading manifest from file GitHub.UI.dll: the manifest may not be valid or the file could not be opened.
• File GitHub.UI.dll is not a valid Portable Executable (PE) file.
• PE file does not have enough data.

COMPONENT STORE TRANSACTION FAILURE SUMMARY
No transaction error was detected.

WARNINGS
There were no warnings during this operation.

OPERATION PROGRESS STATUS

• [18-Dec-13 12:56:05 PM] : Activation of http://github-windows.s3.amazonaws.com/GitHub.application has started.
• [18-Dec-13 12:56:08 PM] : Processing of deployment manifest has successfully completed.
• [18-Dec-13 12:56:08 PM] : Installation of the application has started.
• [18-Dec-13 12:56:14 PM] : Processing of application manifest has successfully completed.
• [18-Dec-13 12:56:19 PM] : Found compatible runtime version 4.0.30319.
• [18-Dec-13 12:56:19 PM] : Request of trust and detection of platform is complete.

ERROR DETAILS
Following errors were detected during this operation.

• [18-Dec-13 02:05:54 PM] System.Deployment.Application.InvalidDeploymentException (ManifestLoad)
• Exception occurred loading manifest from file GitHub.UI.dll: the manifest may not be valid or the file could not be opened.
• Source: System.Deployment
• Stack trace:
  at System.Deployment.Application.Manifest.AssemblyManifest.ManifestLoadExceptionHelper(Exception exception, String filePath)
  at System.Deployment.Application.Manifest.AssemblyManifest.LoadFromInternalManifestFile(String filePath)
  at System.Deployment.Application.Manifest.AssemblyManifest..ctor(String filePath)
  at System.Deployment.Application.DownloadManager.ProcessDownloadedFile(Object sender, DownloadEventArgs e)
  at System.Deployment.Application.FileDownloader.DownloadModifiedEventHandler.Invoke(Object sender, DownloadEventArgs e)
  at System.Deployment.Application.FileDownloader.OnModified()
  at System.Deployment.Application.SystemNetDownloader.DownloadSingleFile(DownloadQueueItem next)
  at System.Deployment.Application.SystemNetDownloader.DownloadAllFiles()
  at System.Deployment.Application.FileDownloader.Download(SubscriptionState subState)
  at System.Deployment.Application.DownloadManager.DownloadDependencies(SubscriptionState subState, AssemblyManifest deployManifest, AssemblyManifest appManifest, Uri sourceUriBase, String targetDirectory, String group, IDownloadNotification notification, DownloadOptions options)
  at System.Deployment.Application.ApplicationActivator.DownloadApplication(SubscriptionState subState, ActivationDescription actDesc, Int64 transactionId, TempDirectory& downloadTemp)
  at System.Deployment.Application.ApplicationActivator.InstallApplication(SubscriptionState& subState, ActivationDescription actDesc)
  at System.Deployment.Application.ApplicationActivator.PerformDeploymentActivation(Uri activationUri, Boolean isShortcut, String textualSubId, String deploymentProviderUrlFromExtension, BrowserSettings browserSettings, String& errorPageUrl)
  at System.Deployment.Application.ApplicationActivator.ActivateDeploymentWorker(Object state)
  --- Inner Exception ---
  System.IO.IOException
• File GitHub.UI.dll is not a valid Portable Executable (PE) file.
• Source: System.Deployment
• Stack trace:
  at System.Deployment.Application.PEStream.ConstructFromFile(String filePath, Boolean partialConstruct)
  at System.Deployment.Application.PEStream..ctor(String filePath, Boolean partialConstruct)
  at System.Deployment.Application.Manifest.AssemblyManifest.LoadFromInternalManifestFile(String filePath)
  --- Inner Exception ---
  System.IO.IOException
• PE file does not have enough data.
• Source: System.Deployment
• Stack trace:
  at System.Deployment.Application.PEStream.PEComponent.ReadData(FileStream file, Int64 position, Type dataType)
  at System.Deployment.Application.PEStream.ResourceDirectory..ctor(ResourceSection resourceSection, FileStream file, Int64 rootResourceAddress, Int64 resourceAddress, Int64 addressDelta, Boolean partialConstruct)
  at System.Deployment.Application.PEStream.ResourceSection..ctor(FileStream file, SectionHeader sectionHeader, Boolean partialConstruct)
  at System.Deployment.Application.PEStream.ConstructPEImage(FileStream file, Boolean partialConstruct)
  at System.Deployment.Application.PEStream.ConstructFromFile(String filePath, Boolean partialConstruct)

COMPONENT STORE TRANSACTION DETAILS
No transaction information is available.

The EXCESS_ICE scheme was only used in one paper, in 2007. This scheme (currently a user_def.h option) adds substantial complexity to the code in several functions. Ensuring that new features are compatible with EXCESS_ICE is laborious and error-prone (similar to the DIST_PREC option).

People who would like to use this feature can always access an archived version of VIC that contains this feature. I propose that we remove this feature from release 5 onwards.

The SUN1999 snow albedo implementation in snow_utility.c appears different from that in the paper mentioned in the same code (see attached figure for the relevant section).

In particular:

• The calculated albedo is only used in Sun1999 for clear sky and is further modified in the case of cloudy skies, something that is not done in VIC
• From the paper, it would appear that the time unit in the deep snow equation should be in hours (delta t / 3600) in the paper - unfortunately I could not quickly find the units for delta t in the paper, but it seems to be in seconds. In the VIC implementation this is converted to days
• In the paper the cutoff for the deep snow formulation is a snow depth of 25 cm. In the VIC formulation this is 2.5 cm

I am not sure how often the SUN1999 option is actually used. But we should either
a. make sure that it is implemented correctly (which it does not seem to be)
b. remove it

If we do keep it, the comment should reflect at the very least that this is not a complete implementation of the version in the paper (and we should make sure that the units, etc are correct).

If it is wrong (as I think it is), please mark it as a bug and keep it open till fixed.

Sun, S., J. Jin, and Y. Xue, A simple snow-atmosphere-soil transfer model, J. Geophys. Res., 104 (D16), 19,587-19,597, 1999, doi:10.1029/1999jd900305.

in vicNl_def.h, REL_HUMID and OUT_REL_HUMID are described in comments as having units of [fraction]. But in initialize_atmos(), REL_HUMID is actually assumed to be a percent, and is divided by 100. So users looking at vicNl_def.h will be misled.

All that needs to happen is to correct the comments. I'd be happy to do this as part of hotfix 4.1.2.i.

When downscaled daily downward shortwave radiation is used as an input to the model, for some reason incoming longwave is calculated assuming clear skies always, and this results in an underestimate of incoming longwave on cloudy days.
The error is likely linked to the estimate of the clear sky fraction in MTCLIM.

OUT_ROOTMOIST does not contain “Root zone soil moisture” as specified on the Internet. It is the moisture in the soil layers containing roots. This makes a difference if a soil layer is only partly occupied by roots.

An example:
most of my upper and middle soil layers have depths of 0.3 and 2.5 m, respectively, while roots occupy the upper layer and the uppermost 70 cm of the middle layer. Hence, OUT_ROOTMOIST should, I would think, contain the moisture from the upper layer + 70/250 times the moisture from the middle layer. In the current VIC output (I have VIC.4.2.1g), OUT_ROOTMOIST contains the moisture from the upper layer plus all of the moisture from the middle layer.

Currently penman() uses LAPSE_PM (6 K/km), while snowbands use T_lapse (6.5 K/km).

Description - Remove initial conditions from parameter files. Separate the cell ID information from soil parameters. Change format to netCDF?
Optional? - Yes
Comment - For semi-backward compatibility, would require tools for parameter file conversion.

See issue #23 for more details

This would include different lapse rates for Tmin and Tmax, as well as seasonal changes in these.

Observations in mountainous areas suggest Tmin and Tmax can deviate substantially from the standard 6.5 K/km.

DHSVM allows for spatial and temporal variation in these lapse rates.

I suggest that we adopt some standard for VIC code formatting for a few reasons:

a) a mixture of tabs and spaces is currently used for code indentation, which depending on the individual users editor settings (tab size, etc.) makes for code that is very hard to read.

b) a lot of the code has trailing whitespace that some editors will auto remove (again depending on user settings). This leads to non-significant whitespace changes in the code, which in turn result in harder to read diffs in version control (since you may have to plow through many whitespace changes).

Rather than coming up with a long list of code format requirements, I suggest using a code formatting tool: uncrustify, which everyone would run with a standard configuration file (which itself will be included in the VIC git repo), before committing changes.

My suggestion is to implement this as part of VIC 5 rather than 4.2, to avoid a gazillion whitespace changes in 4.2. VIC 5 will be a major change anyway. I can make my configuration file available.

Currently, output messages are handled in several ways:

• fprintf()
• fprintf() surrounded by VERBOSE condition
• nerror()
• vicerror()

Ideally, all of these output types would be replaced by a single logging function, which would take as inputs a message string and a status code (see Bart's suggestion in issue #21 comments for more details). Thus, a single function could handle errors, warnings, important information, and less important details. The desired status level for output should be controlled in the global parameter file.

Description - Require forcing files to contain dates/times, and remove this information from the global parameter file.
Optional? - No
Comment - This would be included by using properly formatted netCDF forcings.

See issues #17 and #23 for more details.

Proposal for VIC source code reorganization

What and why?

I propose a reorganization of the VIC code, that would allow multiple VIC drivers to use a single science core, so that science-based changes to the code can easily be shared amongst difference VIC configurations.

VIC is currently implemented in a number of different configurations, both offline and coupled. For example:

• offline, time-first mode ( VIC classic ): This is the configuration used by the code that is distributed by the UW team through its own web site and through the Github repository.
• offline, space-first mode ( VIC image mode): This is the configuration used in the VIC implementation in NASA's Land Information System (LIS), which uses parts of the Earth System Modeling Framework (ESMF).
• coupled, space-first mode ( VIC image mode ): This is the configuration used in the VIC implementation in the Regional Arctic System Model (RASM), which uses the Community Earth System Model (CESM). CESM in turn uses parts of the Model Coupling Toolkit (MCT).

In addition, there are likely many project-specific configurations of VIC in use by other teams.

The challenge with the current implementation is that the computational or scientific core of VIC is not cleanly separated from the driver. The effect of this is that once a VIC version is implemented within a certain configuration, it is difficult to keep that configuration updated with the latest science-based changes that are made to VIC. In other words, after porting VIC version X so that it works in anything other than the default configuration ( VIC classic ), it is difficult to accommodate changes made as part of the VIC versions X.1, X.2, etc.

The goal of the proposed reorganization is limited to cleanly separating the VIC computational / scientific core from the driver. The driver determines whether the model is run in uncoupled, coupled, time-first, or space-first mode. This will allow all configurations to benefit from updates to VIC's core. Note that changes to VIC's core may require additional changes to the driver (for example, if new parameters need to be read).

The proposed reorganization includes changes in the organization of the VIC source code tree and in changes in the source code itself. Note that these changes should not affect the results. That is, VIC classic should produce the same results before and after the reorganization. Similarly, when using the same model parameters and driven with the same meteorological forcings, all drivers should produce the same results. This will allow us to put some rigorous tests in place to ensure that the reorganization does not introduce unwanted effects and to ensure that the results are correct if someone creates a new driver.

The reorganization will allow us to add additional drivers to the VIC code repository, which would allow VIC to be operated in time-first or space-first mode, uncoupled or coupled.

The following are some preliminary ideas (feedback requested) for how to implement this reorganization.

Driver and VIC core separation

Each driver must call at a minimum the following functions:

• vic_alloc() -- Allocate memory for VIC structures
• vic_init() -- Initialize model parameters
• vic_restore() -- Restore model state
• vic_run() -- Run VIC for a single grid cell, for a single time step.
• vic_write() -- Write VIC model output for a single timestep (either for an entire domain or for a single grid cell).
• vic_save() -- Save a VIC model state.
• vic_finalize() -- Final cleanup.

In essence, the most important call will be vic_run(), which will run VIC for a single timestep and for a single grid cell. All the code that is invoked as part of the vic_run() call is by definition part of VIC core and will be the same regardless of the each driver. The arguments to the vic_run() call will include the complete set of meteorological forcings for that particular time step, that is, any manipulation of meteorological forcings will be done in the driver. The memory allocated in vic_alloc() will need to persist in the driver between successive calls to vic_run() until vic_finalize() is called.

The other vic_ functions, as well as any other functionality will be implemented as part of each driver and will consequently vary between VIC implementations, even though there may be a significant amount of overlap between drivers. Note that the drivers will need to implement additional functionality, for example, how to deal with meteorological forcings, how to deal with time varying model parameters that are read from file, and how to deal with changes to a model state as part of a data assimilation scheme.

Because only the code that is invoked by vic_run() is part of VIC's core, it could be argued that only the call to vic_run() should be required by each driver. However, requiring calls to the other vic_ functions as well, will likely promote code reuse among drivers and facilitate the implementation of new drivers. For example, two different drivers that both operate in image mode and write NetCDF output, may be able to use the same vic_write() and vic_save() functions and parts of the same vic_init() and vic_finalize() functions.

In short, pseudo-code for VIC classic would look something like:

foreach gridcell:
    vic_alloc()
    vic_init()
    vic_restore()
    foreach timestep:
        vic_run()
        if output:
            vic_write()
        if save:
            vic_save()
    vic_finalize()

Pseudo-code for an image model implementation would look something like:

vic_alloc()
vic_init()
vic_restore()
foreach timestep:
    foreach gridcell:
        vic_run()
    if output:
        vic_write()
    if save:
        vic_save()
vic_finalize()

In both cases there would be a large amount of additional code between consecutive vic_ calls. This code would be specific to each driver. For example, before each call to vic_run(), atmospheric forcings would need to be updated, time-varying model parameters may need to be updated, and the model state may need to be updated in a data assimilation scheme. Whether predefined names should be used for each of these steps is up for discussion (as is the rest of this document).

NVIC drivers may need to be implemented in Fortran to interact with other model components (for example as part of RASM). In that case it is important that the vic_ functions are callable from Fortran, in particular the vic_alloc(), vic_run(), and vic_finalize() functions. Alternatively, these functions may need to be wrapped in another set of functions that are callable from Fortran. I am not sure yet, how this should be implemented and how driver dependent this will be and how this will affect the above functions.

Source tree organization

In the current version of VIC, the source code is all located in a single directory. I propose that we separate the code into directory trees that refect the code separation outlined above. For example:

|-drivers
|---classic
|-----include
|-----src
|---lis
|-----include
|-----src
|---rasm
|-----include
|-----src
|---test
|-----include
|-----src
|-vic_run
|---include
|---src

For all directories I propose that we cleanly split the header files from the source code (hence include and src directories in each of the subdirectories). All the core vic code would go in the vic_run directories, while all the other code would be specific to each driver. I am thinking that there must be a better way to ensure that code is re-used, so am looking for suggestions. For example, should the mtclim code go in to the driver/classic directory or somewhere else? How can we set it up so that read and write functions can be shared more easily. Alternatively, just keep it split up as above and allow for duplication, since each driver will be tailored to a specific environment and/or application.

As said, this is a proposal and needs work: Let the feedback begin. I am planning to start making changes along these lines on a branch in the next week or two.

The cause appears to be a simple flaw in the logic of parsing the lake parameter file.

VIC crashes on some cells when EXP_TRANS and IMPLICIT are TRUE (and FROZEN_SOIL is TRUE).

It may be that the crash-proofing that we've added in the past didn't cover this combination of options.

I've recently spoken with a user who was using bad parameters for the soil temperature scheme and getting bad results. This probably has happened to other people too, due to VIC's lack of constraint on these parameters.

I'd like to suggest the following changes to VIC's defaults and parameter validation:

1. Make the implicit scheme the default when FROZEN_SOIL is TRUE, rather than the explicit scheme. Unlike the explicit scheme, which is inaccurate under certain combinations of time step length, node spacing, and thermal conductivity, the implicit scheme is fairly robust.
2. Make the exponential node spacing the default option. This places more nodes near the surface, where spatio-temporal gradients are larger, which allows us to get greater accuracy with fewer nodes (= shorter run-times).
3. Add a validation of the number of thermal nodes as a function of the specified bottom boundary depth, such that the user isn't allowed to specify too few nodes.

Will incorporate something like Ingjerd Haddeland's scheme

High canopy evaporation
Simulated evaporation from canopy storage may be unrealistically high for time step = 24 h

Actual evapotranspiration sometimes exceeds potential evapotranspiration
This may be another symptom of the high canopy evaporation for time step = 24h mentioned above

Currently, input and output variable declarations are handled in vicNl_def.h via precompile defines that alias strings like "OUT_RUNOFF" to numbers. The cumbersome part of this is that, if we wish to keep the list organized in some way for ease of finding variables (e.g., alphabetic and/or grouped by module), then adding a new variable in the middle of the list requires renumbering the entire remainder of the list. This is tiresome and error-prone. It also creates a lot of false differences between consecutive versions of vicNl_def.h, obscuring the true functional differences.

When I originally implemented this system, I did so only because my attempt to do this with an enumeration failed. I'm not sure why; it should be possible (as in DHSVM). At this point I think it is worthwhile to investigate the feasibility of switching to an enumeration.

Xu Liang (see below for contact info) has done some work on this
Students in the Lettenmaier lab are also working on this

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.