OPM / opm-parser

http://www.opm-project.org
11 stars 44 forks source link

Summary missing wells #1218

Closed joakim-hove closed 6 years ago

joakim-hove commented 6 years ago

Will handle keywords like:

WWCT
/

in the summary section without complaining - even for decks without wells.

Observe that the second commit implies policy change, now the default behaviour for error condition SUMMARY_UNKNOWN_WELLis to completely ignore it. This is a policy change, the behaviour with respect to the

WWCT
/

configuration does not require this change of policy.

joakim-hove commented 6 years ago

jenkins build this please

joakim-hove commented 6 years ago

jenkins build this please

joakim-hove commented 6 years ago

jenkins build this please

atgeirr commented 6 years ago

I may have forgotten or misunderstood something, but why is the parser default policy changed, instead of only changing the policy used by Flow?

joakim-hove commented 6 years ago

I may have forgotten or misunderstood something, but why is the parser default policy changed, instead of only changing the policy used by Flow?

To be honest I kind-of lost track here myself; was quite a lot of back and forth. The whole situation originated in the following:

  1. The summary section contains the keyword:

    WWCT
    /

    which means that you want WWCT output for all wells.

  2. In order to check stability of the initial solution people remove all wells from the Schedule file and simulate for a period.

When the current implementation of flow meets the no-wells-schedule file there is a:

if (wells.empty())
   handleMissingWells( )

code block which will trigger the SUMMARY_UNKNOWN_WELL error condition. From experienced users in the local shop it was deemed unacceptable to treat this situation as anything out of the ordinary. The third commit: 9d65bffcaacb1508 is sufficient to fix that.

As part of the discussion it was suggested to change the default behaviour when it comes to unknown wells, but observe that such a change is not necessary to fix the specific error condition which triggered this. If we indeed decide to change behaviour it can be done like in this PR - or alternatively it can be done in flow.

The current policy change involves a departure from the: "All errors willl by default raise an exception" policy, which I guess the most significant change? As I see it the second commit here is the controversial one - and we can:

  1. Completely remove it.
  2. Merge it as is.
  3. Remove it from opm-parser, but implement similar behaviour through the initialization in flow.

My preference would be to go with 1). Opinions @alfbr?

alfbr commented 6 years ago

My preference would be to go with 1). Opinions @alfbr?

This probably needs both consideration and discussion. I am on vacation this week, so I hope we can catch up on long term solution next week, Until then, your judgment is the best we have.

joakim-hove commented 6 years ago

jenkins build this please

joakim-hove commented 6 years ago

This probably needs both consideration and discussion. I am on vacation this week, so I hope we can catch up on long term solution next week, Until then, your judgment is the best we have.

OK - then I will merge without the policy changing second commit; we can come back to that. The original problem report will be fixed with the current merge.

joakim-hove commented 6 years ago

jenkins build this please