Introduced the CriticalError exception class.

[kjetilly]

I have added a simple exception class CriticalError which is supposed to wrap normal exceptions and rethrow them as critical exceptions where errors can not be recovered easily.

This is relevant for the linear solver system where a caught exception is typically treated by lowering time steps. This leads to the unfortunate side effect that some errors^* in the JSON file will be treated by reducing time steps until the limit is reached and the simulation is crashed, with few error messages to the actual problem that caused the problem.

One could also add a constructor that only takes an error message to let coders directly throw a critical error message.

^* Setting --linear-solver to point to the following json will cause this behaviour:

{
    "preconditioner": "ParOverILU0"
}

since ISTLSolver::initPrepare tries to access "preconditioner.type".

See the opm-simulators PR 5969 on this can be used.

[kjetilly]

Jenkins build this opm-simulators=5969 please

[kjetilly]

Jenkins build this opm-simulators=5969 please

[kjetilly]

Jenkins build this opm-simulators=5969 please

[kjetilly]

Jenkins build this opm-simulators=5969 please

[kjetilly]

Jenkins build this opm-simulators=5969 please

[kjetilly]

Jenkins build this opm-simulators=5969 please

[atgeirr]

I think that the class proposed here looks good! (Even the default function argument in the constructor...) One could argue that it should inherit std::runtime_error, but we can make that change later if we decide it is more appropriate.

However, do not like the macros here, as currently presented.

One minor problem is that it is not trivial to make automatic formatting indent after OPM_BEGIN_TRY_CATCH_RETHROW_AS_CRITICAL_ERROR. We could drop the brace, or just always insert a brace anyway (extra pair of braces around a scope is harmless). Incidentally that is what we do with OPM_BEGIN_PARALLEL_TRY_CATCH, I was not aware of that minor flaw before now.

More serious is that it obscures what goes on, and makes it "too easy" to just use the macro instead of writing a proper message. Case in point, in the downstream PR, there should be a message similar to "Invalid property tree for linear solver setup in file xxx.json. Error was what()-string-from-json-lib" instead of the message created by the macro. In general, messages should tell the user what needs to be corrected rather than the file and line where it happened.

I suggest to drop the macro as written here, and instead just do a regular try-catch at the site needed. It is ok to add a macro for throwing the CriticalError if you want, but it is not something we need until we add this to multiple sites, and we may only realize then what the most useful macro will be.

Edit: A simple utility free function can probably do most of the work of the macro here, if we agree to drop the file and line number.

[kjetilly]

The point of the macro is to guard certain functions, such as the one found in the opm-simulators PR, in the sense that errors from this function is not easily recoverable.

Indeed, the code that generates the error does no checking and at the place we rethrow the error, we do not know if the error came from bad formatting of the the JSON file, that is just one possibility. In other words, it would be difficult to make a more descriptive error message at that part of the code.

For the JSON error in itself, one should of course check for the existence of "preconditioner.type" before accessing it and if it does not exist, throw a CriticalError. However, that is not the main point of the opm-simulator PR, it was simply to add a barrier that remade any error done in the prepare-function into a CriticalError.

[kjetilly]

I modified the throw macro a bit to enhance its utility: Now any CriticalError is simply rethrown and not wrapped, so it will allow parts deeper in the stack to throw a CriticalError that is simply passed along this throw macro.

The point being that now it has three catch-clauses, with potential more coming (maybe we want to add something special for DUNE errors?) so I think it is worth keeping as a macro.

[kjetilly]

Jenkins build this opm-simulators=5969 please

[kjetilly]

Jenkins build this opm-simulators=5969 please

[kjetilly]

Jenkins build this opm-simulators=5969 please

[atgeirr]

This and the downstream PR looks good to me now, merging.