Get rid of user_def.h about vic HOT 16 CLOSED

Ted to scope out what is easy to remove (these will become part of 4.2) and what is hard (VIC 5.0). This issue will likely be split into multiple new issues (at which time this one can be closed).

from vic.

Here are my recommendations. In all cases, we can do something for 4.2, so that user_def.h potentially could be removed for 4.2. My recommendations are for 4.2 unless otherwise stated.

from vic.

Note: the contents of snow.h could also potentially be moved to vicNl_def.h; the contents of LAKE.h could be moved to vicNl.h; and some/all of the contents of global.h could be moved to vicNl_def.h. This is mainly for ease of dealing with the code.

from vic.

Ease is good. However, if they are functionally different pieces of the code it might be good to keep them separate. Originally snow.h was separate, because it was basically the same snow code as DHSVM and it made it easier to upgrade compare. These are a bit different than the user_def.h cases and I think a case could be made to keep them separate.

Bart

On Dec 18, 2013, at 10:12 AM, Ted Bohn [email protected] wrote:

Note: the contents of snow.h could also potentially be moved to vicNl_def.h; the contents of LAKE.h could be moved to vicNl.h; and some/all of the contents of global.h could be moved to vicNl_def.h. This is mainly for ease of dealing with the code.

—
Reply to this email directly or view it on GitHub.

from vic.

If there are no objections to my recommendations, I can go ahead and start making those changes.

Let me know if you would like me to break any of the individual options out into separate issues (and assign them to myself).

from vic.

As mentioned - I am not sure that we want to get rid of snow.h at this time, but please proceed with user_def.h.

On Dec 22, 2013, at 3:41 PM, Ted Bohn [email protected] wrote:

If there are no objections to my recommendations, I can go ahead and start making those changes.

Let me know if you would like me to break any of the individual options out into separate issues (and assign them to myself).

—
Reply to this email directly or view it on GitHub.

from vic.

@tbohn, at a minimum, lets try to put each change to user_def.h variables in their own commits. They all can live under this issue.

from vic.

A couple questions:

1. Joe and Bart, before I remove QUICK_FS, is there any chance you'd want to use this feature in your RACM project? I know you want to simulate frozen soil and that runtime is an issue. Keep in mind that this feature has never been fully tested (according to the feature's own comment statements). So, to use it, I guess we'd need to test it to verify whether it works.
2. Regarding VERBOSE: another possible solution could be to keep it (as a run-time option, rather than compile-time) and move it into a wrapper function (similar to nrerror) that checks its value before printing. The hard part would be going through the code and replacing all printing statements with a single function. This might be worth breaking out into a separate issue, that includes reconciling vicerror() and nrerror(), perhaps creating a single printing function that handles all "log" output?

from vic.

1. I think it QUICK_FS can be removed, especially if it has not been tested. We are not using it right now and runtime is not really an issue for our coupled arctic modeling since we run on many processors. We are not using it in the current version that is coupled in RASM.
2. Yes - I would break it into a separate issue. We'd probably want some log function that uses a log level that can be set by the use at run-time to determine the verbosity for the amount of detail to be logged. For example, analogous to pythons logging module (http://docs.python.org/2/library/logging.html):

CRITICAL 50
ERROR 40
WARNING 30
INFO 20
DEBUG 10

Bart

On Dec 26, 2013, at 4:50 PM, Ted Bohn [email protected] wrote:

A couple questions:

1. Joe and Bart, before I remove QUICK_FS, is there any chance you'd want to use this feature in your RACM project? I know you want to simulate frozen soil and that runtime is an issue. Keep in mind that this feature has never been fully tested (according to the feature's own comment statements). So, to use it, I guess we'd need to test it to verify whether it works.
2. Regarding VERBOSE: another possible solution could be to keep it (as a run-time option, rather than compile-time) and move it into a wrapper function (similar to nrerror) that checks its value before printing. The hard part would be going through the code and replacing all printing statements with a single function. This might be worth breaking out into a separate issue, that includes reconciling vicerror() and nrerror(), perhaps creating a single printing function that handles all "log" output?

—
Reply to this email directly or view it on GitHub.

from vic.

OK, how about this: I will handle VERBOSE in 2 stages. The first stage will simply be to transfer the VERBOSE precompile definition from user_def.h to vicNl_def.h, so that user_def.h can be removed and this issue can be closed. For the second stage, I'll create a separate issue to create consistent logging. VERBOSE will go away as part of the second stage. Any objections?

from vic.

No objections

On Dec 27, 2013, at 4:31 PM, Ted Bohn [email protected] wrote:

OK, how about this: I will handle VERBOSE in 2 stages. The first stage will simply be to transfer the VERBOSE precompile definition from user_def.h to vicNl_def.h, so that user_def.h can be removed and this issue can be closed. For the second stage, I'll create a separate issue to create consistent logging. VERBOSE will go away as part of the second stage. Any objections?

—
Reply to this email directly or view it on GitHub.

from vic.

One last question. Regarding NO_REWIND, I suggested two options: either move it to the options_struct in vicNl_def.h as a run-time option, or getting rid of it altogether. I'm leaning towards getting rid of it, since it seems like our parameter files are always set up so that all grid cells are present and appear in the same order. It would just fall on the user to set up their files correctly. Any objections?

from vic.

As long as the program aborts if it cannot find the next grid cell that is fine with me. I'd like to move my parameter files to netcdf anyway (eventually), where this would not be an issue. We may also write a little preprocessir script that properly checks (and reorders the files if need be).

Bart

On Dec 28, 2013, at 12:23 PM, Ted Bohn [email protected] wrote:

One last question. Regarding NO_REWIND, I suggested two options: either move it to the options_struct in vicNl_def.h as a run-time option, or getting rid of it altogether. I'm leaning towards getting rid of it, since it seems like our parameter files are always set up so that all grid cells are present and appear in the same order. It would just fall on the user to set up their files correctly. Any objections?

—
Reply to this email directly or view it on GitHub.

from vic.

These changes are now complete. User_def.h is now gone (for details, see my pull request). I am creating 2 new issues (each one much smaller than this one was):

• Consistent logging function (to replace the VERBOSE option and nerror and vicerror functions) (issue #64)
• Consider removing the LOG_MATRIC option (was LOW_RES_MOIST) - preliminary tests indicate that it has very little effect (issue #65)

from vic.

This code update also addresses issue #38.

from vic.

Now that the pull request has been executed, I'm closing this issue.

from vic.