OPM / opm-core

Collection of utilities, solvers and other components.
http://www.opm-project.org
GNU General Public License v3.0
44 stars 50 forks source link

test_RelpermDiagnostics made to use PARSE_EXTRA_RECORDS. #1210

Closed stefoss23 closed 6 years ago

stefoss23 commented 6 years ago

The test RelpermDiagnostics needs to set the PARSE_EXTRA_RECORDS field instead of the PARSE_RANDOM_TEXT field in order to comply with with pull request opm-parser#1187.

joakim-hove commented 6 years ago

Unless the purpose of this test is to actually provoke with slightly broken input, my suggestion would rather be to fix the input deck and use fully strict default ParseContext; if the broken input should be retained this fix looks correct.

atgeirr commented 6 years ago

jenkins build this please

joakim-hove commented 6 years ago

Ehhh - @stefoss23 and myself have agreed that this is the wrong approach; the data file used in the testing should be tidied up and the parsing should be done in strict mode.

Unless the purpose of this test is to actually provoke with slightly broken input

That is probably the case - but that "brokenness" is on a higher semantic level than the parsing. The extra record in the SGOF table - which is the culprit here, will never actually reach Deck datastructure independently of how things are parsed.

atgeirr commented 6 years ago

Ehhh - @stefoss23 and myself have agreed that this is the wrong approach; the data file used in the testing should be tidied up and the parsing should be done in strict mode.

Yes, to me it looked like that was the current state of this PR, which is why I triggered a Jenkins run. Is the required change different from what is here now? I see that I also forgot to include the upstream PR in the command, so we'll have to rerun anyway.

joakim-hove commented 6 years ago

jenkins build this please

akva2 commented 6 years ago

please choose one alternative: 1) this will land now 2) will reopen against opm-simulators after the rest of opm-core has been imported there

i need a frozen state as the history filtering will take some iterations and if something new is merged here it makes the process harder.

joakim-hove commented 6 years ago

I take 1)