uw-hydro / vic Goto Github PK
View Code? Open in Web Editor NEWThe Variable Infiltration Capacity (VIC) Macroscale Hydrologic Model
Home Page: http://vic.readthedocs.io
License: MIT License
The Variable Infiltration Capacity (VIC) Macroscale Hydrologic Model
Home Page: http://vic.readthedocs.io
License: MIT License
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.
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
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.
This would be very useful for climate forcings that have a "no_leap" calendar. LEAPYR is defined at the top of make_dmy.c.
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.
Xu Liang (see below for contact info) has done some work on this
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:
read_soilparam.c
to read_soilparam_arc.c
. This would also require testing.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.
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.
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
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.
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.
Description - Use netCDF library to support binary I/O.
Optional? - No
Comment - Includes: state files, history files, parameter files, and forcings.
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.
See later
Xu Liang (see below for contact info) has done some work on this
Students in the Lettenmaier lab are also working on this
Description - Either remove these options or move them to global parameter file. Remove #ifdef’s from code. Remove VIC’s –o flag.
Optional? - No
The cause appears to be a simple flaw in the logic of parsing the lake parameter file.
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.
Currently penman() uses LAPSE_PM (6 K/km), while snowbands use T_lapse (6.5 K/km).
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 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
COMPONENT STORE TRANSACTION FAILURE SUMMARY
No transaction error was detected.
WARNINGS
There were no warnings during this operation.
OPERATION PROGRESS STATUS
ERROR DETAILS
Following errors were detected during this operation.
COMPONENT STORE TRANSACTION DETAILS
No transaction information is available.
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
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.
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.
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
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?
Description - Remove all loops over dist and dist array dimensions from code.
Optional? - No
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.
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.
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).
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:
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.
Description - Linear interpolation of monthly LAI
Optional? - Yes
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.
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:
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.
Each driver must call at a minimum the following functions:
vic_alloc()
-- Allocate memory for VIC structuresvic_init()
-- Initialize model parametersvic_restore()
-- Restore model statevic_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.
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.
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.
Description - Photosynthesis, soil respiration
Optional? - Yes
Comment - Requires additional veg library parameters
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.
Keith Cherkauer and Alan Hamlet have both done some work on this.
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:
Description - Ability to read vegetation parameters for each vegetation type from file at a daily time step
Optional - yes
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
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.
Description - Accounts for wetland microtopography
Optional? - Yes
Needs modification:
Will incorporate something like Ingjerd Haddeland's scheme
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.
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.
Currently, output messages are handled in several ways:
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.
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?
A declarative, efficient, and flexible JavaScript library for building user interfaces.
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google ❤️ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.