Consistent logging function

[tbohn]

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.

[jhamman]

I've had to make a few changes to logging functions / calls as part of building the testing module (#79). That got me thinking about how to clean up the logging in VIC. This link seems to lay out one option that could work for us: http://c.learncodethehardway.org/book/ex20.html. Maybe you guys have other thoughts?

[bartnijssen]

That looks fine to me. The other option would be log4c (http://log4c.sourceforge.net), which I think is a bit more of an actual logging library (logging to files, etc. rather than just printing to stderr), but I like the low overhead of the macros that you are pointing to.

In some ways, VIC's error handling is even more lightweight, in that for any real error, the model should DIE rather than try to recover (it'd be different if we were putting together a plotting server or something like it). That means that the "goto error" part is strictly not even needed. I remember implementing something like that (with the "goto error" which is placed after the return statement at the bottom of the function) at some point, but I suggest for VIC we just crash rather than try to close down cleanly (close files, release memory, etc.).

Would you plain to replace all the error checking with this or for now just replace all the vicerror(), nrerror() and fprintf(stderr) calls?

On Nov 18, 2014, at 1:39 PM, Joe Hamman notifications@github.com wrote:

I've had to make a few changes to logging functions / calls as part of building the testing module (#79). That got me thinking about how to clean up the logging in VIC. This link seems to lay out one option that could work for us: http://c.learncodethehardway.org/book/ex20.html. Maybe you guys have other thoughts?

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

[jhamman]

I agree we want to ovoid recovering from an error. Crashing with an informative error message is where I'd like to point us.

Certainly replacing the existing error and print statements would be a start but I wouldn't rule out the "goto error" method either. I don't have strong feelings about how to do this (I just stumbled upon this so I was just throwing it out there).

[tbohn]

Just to make sure you know the background: the CONTINUEONERROR option (TRUE
by default) was motivated several years ago by the desire to NOT die on
error. But the reason behind that was that VIC was crashing a lot in
scattered grid cells in the frozen soils function; people wanted to be able
to get the 95% of the results that didn't crash. Those crashes have long
since been fixed, so perhaps CONTINUEONERROR is no longer needed...

On Tue, Nov 18, 2014 at 1:59 PM, Bart Nijssen notifications@github.com
wrote:

That looks fine to me. The other option would be log4c (
http://log4c.sourceforge.net), which I think is a bit more of an actual
logging library (logging to files, etc. rather than just printing to
stderr), but I like the low overhead of the macros that you are pointing
to.

In some ways, VIC's error handling is even more lightweight, in that for
any real error, the model should DIE rather than try to recover (it'd be
different if we were putting together a plotting server or something like
it). That means that the "goto error" part is strictly not even needed. I
remember implementing something like that (with the "goto error" which is
placed after the return statement at the bottom of the function) at some
point, but I suggest for VIC we just crash rather than try to close down
cleanly (close files, release memory, etc.).

Would you plain to replace all the error checking with this or for now
just replace all the vicerror(), nrerror() and fprintf(stderr) calls?

On Nov 18, 2014, at 1:39 PM, Joe Hamman notifications@github.com
wrote:

I've had to make a few changes to logging functions / calls as part of
building the testing module (#79). That got me thinking about how to clean
up the logging in VIC. This link seems to lay out one option that could
work for us: http://c.learncodethehardway.org/book/ex20.html. Maybe you
guys have other thoughts?

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

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

[bartnijssen]

You really only need the "goto error" method if you want to recover. Otherwise you can just exit ...

On Nov 18, 2014, at 2:24 PM, Joe Hamman notifications@github.com wrote:

I agree we want to ovoid recovering from an error. Crashing with an informative error message is where I'd like to point us.

Certainly replacing the existing error and print statements would be a start but I wouldn't rule out the "goto error" method either. I don't have strong feelings about how to do this (I just stumbled upon this so I was just throwing it out there).

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

[bartnijssen]

Yes - I am aware of that option (and the problems that people then had in converting incomplete time series to netcdf). I think it has served its purpose in the past, but if it falls by the wayside as part of the logging upgrade that is fine by me. This is basically something that is specific to the driver. It does not work in image mode anyway (incomplete fields?!), but maybe should be maintained in classic mode. We can probably modify the check() or log_error() macros in that case to not exit but go to the next grid cell.

On Nov 18, 2014, at 2:25 PM, Ted Bohn notifications@github.com wrote:

Just to make sure you know the background: the CONTINUEONERROR option (TRUE
by default) was motivated several years ago by the desire to NOT die on
error. But the reason behind that was that VIC was crashing a lot in
scattered grid cells in the frozen soils function; people wanted to be able
to get the 95% of the results that didn't crash. Those crashes have long
since been fixed, so perhaps CONTINUEONERROR is no longer needed...

On Tue, Nov 18, 2014 at 1:59 PM, Bart Nijssen notifications@github.com
wrote:

That looks fine to me. The other option would be log4c (
http://log4c.sourceforge.net), which I think is a bit more of an actual
logging library (logging to files, etc. rather than just printing to
stderr), but I like the low overhead of the macros that you are pointing
to.

In some ways, VIC's error handling is even more lightweight, in that for
any real error, the model should DIE rather than try to recover (it'd be
different if we were putting together a plotting server or something like
it). That means that the "goto error" part is strictly not even needed. I
remember implementing something like that (with the "goto error" which is
placed after the return statement at the bottom of the function) at some
point, but I suggest for VIC we just crash rather than try to close down
cleanly (close files, release memory, etc.).

Would you plain to replace all the error checking with this or for now
just replace all the vicerror(), nrerror() and fprintf(stderr) calls?

On Nov 18, 2014, at 1:39 PM, Joe Hamman notifications@github.com
wrote:

I've had to make a few changes to logging functions / calls as part of
building the testing module (#79). That got me thinking about how to clean
up the logging in VIC. This link seems to lay out one option that could
work for us: http://c.learncodethehardway.org/book/ex20.html. Maybe you
guys have other thoughts?

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

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

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

[jhamman]

My vote would be to use the simpler, macro based, approach. I went ahead and tried out this approach with some simple tests in VIC and it works like a charm (#165).

I think we can cleanly sort out the error handling with regards to the "goto" functionality and the CONTINUEONERROR option.

[jhamman]

closed via #173