OPM / opm-meeting20

Community planning for OPM Meeting 2020
1 stars 0 forks source link

Model for: Error messages and warnings #3

Open joakim-hove opened 4 years ago

joakim-hove commented 4 years ago

Configure behavior with errors and warnings reaching the user. Currently the action when a problem is encountered is typically handled at locally at the problematic site, and to a large extent the final outcome depends on the good taste of the currently active developer. Possible actions include:

The problems with this include:

  1. It is inconsistent for the user.
  2. It is impossible / difficult for the user to configure the behavior.

I suggest creating a model for configurable error handling inspired by the ParseContext class in opm-common[1] which can be used throughout the simulator to query error behavior. In addition to the existing actions currently managed by the ParseContext we should - at least - have some counter based system which will promote a warning to an error and things like that. The code will need to interact closely with the log system and the messagelimiter.

The obvious disadvantage of this is that a myriad of functions will need to get an additional ErrorConfig object passed as a new argument.

[1]: Actually I would suggest renaming the ParseContextclass to e.g. ErrorConfig and then extend that.

atgeirr commented 4 years ago

I think the distributed nature of error logging is necessary, since the seriousness of an error condition must be evaluated by the developer coding it in any case. Printing to cerr or cout directly should be disallowed except in very special circumstances (such as before or after the log system is available). We are therefore left with two choices for the developer:

To adopt the "ParseContext way" taken to an extreme point would mean adding a configuration point for every possible error condition, at a centralized location (unless we get very clever). That is an additional burden on the developer, and I do not think it is very useful. A more moderate approach, where we identify some broad classes of error conditions (similar to the existing use of the ParseContext) and deal with them appropriately can be useful, but I think the choices to be made by the developer become even harder. The choices are then:

To me this looks quite a bit more complicated, and likely to lead to more inconsistencies, not less. Therefore I am sceptical. But I could be convinced of the benefit. Right now I think the parser benefits from the ParseContext approach because the possible errors are well classified, and that it is not uncommon that one can recover and continue by ignoring the condition. Examples of error handling that would benefit from this will be appreciated (and necessary to make me change my mind...).

Final two cents: we should call it ErrorHandlingConfig rather than just ErrorConfig.

joakim-hove commented 4 years ago

Final two cents: we should call it ErrorHandlingConfig rather than just ErrorConfig.

OK - you get the name; and I get the feature?!

joakim-hove commented 4 years ago

To me this looks quite a bit more complicated, and likely to lead to more inconsistencies, not less.

First of all: I symphatise with all your objections - except one. The purpose of this is not to make the developer experience better - but rather the user experience better. But of course - it is important to play along with the developers, we are not only in it for the beer.

Distributed nature: Good point!