OPM / opm-simulators

Simulator programs and utilities for automatic differentiation.
http://www.opm-project.org
GNU General Public License v3.0
111 stars 122 forks source link

2019-04-RC1 Command line Option ecl-strict-parsing=true Error? #1798

Closed OPMUSER closed 5 years ago

OPMUSER commented 5 years ago

Setting the command line option ecl-strict-parsing=true should collect all the errors and print them out before terminating. This is not working, no error messages are produced with this setting - the job just stops.

Setting the option to false, prints the first error and then OPM Flow terminates, as expected.

Secondly, I would suggest the default should be true for producing all the error messages, rather than false.

joakim-hove commented 5 years ago

Setting the command line option ecl-strict-parsing=true should collect all the errors and print them out before terminating. This is not working, no error messages are produced with this setting - the job just stops.

That was frustrating - it works for me; and I have also received reports from others that it works as intended. The error messages go to stderr - could it be that you do not see stderr messages by default in your environment?

Secondly, I would suggest the default should be true for producing all the error messages, rather than false.

Well - the current strict parsing is really strict - setting it to default on we will have endless "Oh - not that strict ...." discussions - for instance Norne as found in the opm-tests repo will not validate in this mode. But maybe/hopefully in the future we can find a good middle ground, we are not there yet though.

OPMUSER commented 5 years ago

I'm confused. If the default is used I get the error message to the terminal, if I set to true I get no messages. From a users prospective I should expect the output to be the same, that is to the terminal. So can we not do this within OPM Flow to avoid confusion? I know I can use re-direct, but if I'm having this problem other REs will have also.

Concerning the level of strictness, perhaps we can use level of strictness, 0, 1 and 2 etc. But that is for another day.

joakim-hove commented 5 years ago

Can you show the verbatim commandline you use.

OPMUSER commented 5 years ago
david@EIPC02-VirtualBox:/media/sf_D_DRIVE/Linux/OPM/LGR$ flow --ecl-strict-parsing=true OPM-LGREPS-ASCII
**********************************************************************
*                                                                    *
*                      This is flow 2019.04-rc1                      *
*                                                                    *
* Flow is a simulator for fully implicit three-phase black-oil flow, *
*             including solvent and polymer capabilities.            *
*          For more information, see https://opm-project.org         *
*                                                                    *
**********************************************************************

Reading deck file 'OPM-LGREPS-ASCII.data'
Failed to create valid EclipseState object.
Exception caught: 
Failed to parse keyword: CARFIN
Starting at location: /media/sf_D_DRIVE/Linux/OPM/LGR/OPM-LGREPS-ASCII.data(62)

Inner exception: Malformed integer ''LGR-2''

terminate called after throwing an instance of 'std::invalid_argument'
  what():  
Failed to parse keyword: CARFIN
Starting at location: /media/sf_D_DRIVE/Linux/OPM/LGR/OPM-LGREPS-ASCII.data(62)

Inner exception: Malformed integer ''LGR-2''

Aborted (core dumped)
david@EIPC02-VirtualBox:/media/sf_D_DRIVE/Linux/OPM/LGR

I'm using this deck to test the feature, the deck should fail because there are two LGRs and various LGR keywords that are not recognize by OPM Flow.

I originally I used OPMRUN to test, but it has the same pro OPM-LGREPS-ASCII.DATA.zip

blem.

joakim-hove commented 5 years ago

Ok thank you, I can not look into it now, but: there are errors and there are errors. Only the errors which are treated with the ParseContext error recovery mechanism give the behaviour you want - this is (probably) nit.

joakim-hove commented 5 years ago

OK - the CARFIN keyword was incorrectly configured - that is fixed here: https://github.com/OPM/opm-common/pull/726 - of course LGRs and the CARFIN keyword does not work in any way in OPM/flow - so the fix only affects the very initial part of the parsing.

OPMUSER commented 5 years ago

One less bug to worry about :-)

OPMUSER commented 5 years ago
david@EIPC02-VirtualBox:/media/sf_D_DRIVE/Linux/OPM/LGR$ flow --ecl-strict-parsing=true OPM-LGREPS-ASCII
**********************************************************************
*                                                                    *
*                      This is flow 2019.04-rc1                      *
*                                                                    *
* Flow is a simulator for fully implicit three-phase black-oil flow, *
*             including solvent and polymer capabilities.            *
*          For more information, see https://opm-project.org         *
*                                                                    *
**********************************************************************

Reading deck file 'OPM-LGREPS-ASCII.data'
Failed to create valid EclipseState object.
Exception caught: 
Failed to parse keyword: CARFIN
Starting at location: /media/sf_D_DRIVE/Linux/OPM/LGR/OPM-LGREPS-ASCII.data(62)

Inner exception: Malformed integer ''LGR-2''

terminate called after throwing an instance of 'std::invalid_argument'
  what():  
Failed to parse keyword: CARFIN
Starting at location: /media/sf_D_DRIVE/Linux/OPM/LGR/OPM-LGREPS-ASCII.data(62)

Inner exception: Malformed integer ''LGR-2''

Aborted (core dumped)
david@EIPC02-VirtualBox:/media/sf_D_DRIVE/Linux/OPM/LGR

I'm using this deck to test the feature, the deck should fail because there are two LGRs and various LGR keywords that are not recognize by OPM Flow.

I originally I used OPMRUN to test, but it has the same pro [Uploading OPM-LGREPS-ASCII.DATA.zip…]()

blem.

joakim-hove commented 5 years ago

I don't know why you reopened this - but observe that the fix: OPM/opm-common#726 is not included in the RC1 release; the release manager (@akva2) could choose to include it in the final release - but as it does not really fix all-that-much it is maybe not very much point.

OPMUSER commented 5 years ago

@joakim-hove we have the similar problem with the analytical and numerical aquifer keywords, see the attached files.

2019-04-19 16_21_45-Ubuntu-Mate-18 04 3 TLS OPM 2019-04-RC1  Running  - Oracle VM VirtualBox

AQFA-OPM-FMT.zip AQFN-OPM-FMT.zip

I'm attempting to write up the documentation for OPM Flow file formats, so I have to generate some cases to see what the files look like. So I thought this also might help the debugging of the parser as well.

OPMUSER commented 5 years ago

Same problem different keywords.

joakim-hove commented 5 years ago

Same problem different keywords.

You might see the same symptoms, but the underlying problem might be completely different; for me as a developer the best is if you just report the problem - with as much detail as possible - without making claims of what the problem is.

Anyway I have briefly looked at the two decks; for AQFA-OPM-FMT there is an unknown keyword, I get a reasonable error message when parsing that:

bash% flow --ecl-strict-parsing=true AQFA-OPM-FMT.DATA
**********************************************************************
*                                                                    *
*                      This is flow 2019.04-pre                      *
*                                                                    *
* Flow is a simulator for fully implicit three-phase black-oil flow, *
*             including solvent and polymer capabilities.            *
*          For more information, see https://opm-project.org         *
*                                                                    *
**********************************************************************

Reading deck file 'AQFA-OPM-FMT.DATA'

Errors:
  PARSE_UNKNOWN_KEYWORD: Unknown keyword: FLUXTYPE

The AQFN deck uses the string O instead of OPEN for the well status in the COMPDAT keyword, flow will look for the strings 'OPEN', 'SHUT', 'AUTO' and 'STOP' - since 'O' is not recognized it barfs - with a reasonable error message.

Might be that Eclipse accepts shorter forms like 'O' - but that is at least not documented.

OPMUSER commented 5 years ago

Thanks @joakim-hove. I'll close this, sorry for the delay in responding.