Fix corpus false positives in deck diagnostics - #53
Merged
Conversation
The line following TITLE is free-form text, but the diagnostics engine scanned it as a keyword. When that text happened to be an upper-case token matching a real keyword name (CO2STORE, H2STORE) or an arbitrary label (ACTIONX_GCONPROD, PYACTION_GCONPROD_INSERT_KW), it was wrongly flagged as an indented or unrecognised keyword. Consume the single free-form text line after any raw-text keyword (TITLE) verbatim, before section/terminator/keyword analysis.
opm-common classifies some keywords as 'fixed' without a concrete record count (EOS derives its count from another keyword). The engine read that as 'expects zero records', so the value record on the next line (EOS / PR) was mistaken for a new keyword and PR was flagged as unrecognised. When a fixed keyword has no records_meta/size_count but does declare a per-record column count, assume it expects at least one record.
Probe-expanded SUMMARY vectors (WSIR, WSPR, WMCTL, and the C/W component-rate vectors) are classified 'array' but, unlike the manual-derived mnemonics, lack the optional_body flag. Written bare and stacked under a single shared '/', they were wrongly flagged as missing their own terminating '/'. Treat any array-kind keyword whose sections include SUMMARY as optional-body when it consumed no records, keying off the SUMMARY section so real cell arrays (PRESSURE, PORO) still require their '/'.
RPTRST/RPTSCHED/RPTSOL/… take a list of output mnemonics terminated by '/'. Many mnemonics are spelled like real keywords in column 1 (PRESSURE, SGAS, SOIL, XMF, YMF, ZMF), so the scanner closed the report block early and flagged each as a wrong-section / unterminated keyword. While a report keyword's block is open, treat every column-1 token as body content; a section header still ends the block. RPTSCHED is dropped from the default exclusion list since the body is now parsed correctly without suppressing checks on the keyword itself.
Some keywords OPM Flow accepts are in neither the reference manual nor opm-common, so they never reach the generated keyword index and were flagged as unrecognised on the known-good corpus — and where a missing keyword swallowed its own records, the well names under it cascaded into further false positives. Add a small hand-maintained supplement (CO2STORE/H2STORE/thermal/ compositional keywords FGDN, FCGMM/I, FCWM, WCMPR/IR, WELLSHUT, STORE, AIM, CVTYPE, AMF, PREFT, ZCRITVIS, SPECHA-H) merged into the index by both the extension and the corpus harness. WELLSHUT is list-shaped so it absorbs its well-name records; the SUMMARY vectors carry section/shape; the rest are recognised-only.
opm-common's probe expansion (and the L-modifier variants CGMIRL, CGMPRL, …) emit recognised SUMMARY entries with no record shape, so their optional '/'-terminated name list (CGMIRL / INJ1 / /) was parsed with the well name mistaken for a new keyword. Normalise every SUMMARY-section vector that lacks a size_kind to the 'array' shape (the bare case is already exempt from the terminator check). Guard the array name-list absorption so a column-1 token that is itself a recognised SUMMARY vector by shape (UDQ name, region-set, L-modifier, deck_name_regex family) starts its own vector rather than being swallowed — otherwise a bare enable-keyword (PERFORMA) absorbs the UDQ mnemonics that follow it and is then flagged for a missing terminator. Both loaders now share prepareKeywordIndex (supplement + normalisation).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Running the diagnostics engine over the known-good OPM/opm-tests corpus surfaced 63 suspected false positives across 21 decks. Every deck there parses and runs in OPM Flow, so each diagnostic was a bug, a missing index keyword, or a parsing gap. This branch fixes the confidently-correct ones, taking the corpus from 63 → 7 (89% reduction). Each fix is an individual commit with focused unit tests; the full suite (333 tests) is green.
Fixes
TITLEis free text but was scanned as a keyword; when it collided with a real keyword name (CO2STORE,H2STORE) or an arbitrary label (ACTIONX_GCONPROD,PYACTION_GCONPROD_INSERT_KW) it was flagged. Now consumed verbatim.EOS) asfixedwithout a concrete record count, so the engine read "expects zero records" and flagged the value record (EOS/PR) as unknown. Now assumes ≥1 record when a per-record column count exists.WSIR,WSPR,WMCTL, component-rate vectors) lack theoptional_bodyflag and were flagged for a missing/when written bare and stacked. Now exempt when they consumed no records, keyed off the SUMMARY section so real cell arrays (PRESSURE,PORO) still require their/.RPTRST/RPTSCHED/… take a free-form mnemonic list, many spelled like real column-1 keywords (PRESSURE,SGAS,SOIL,XMF, …). The scanner closed the block early and flagged each as wrong-section/unterminated. Their bodies are now understood directly;RPTSCHEDdrops off the blanket exclusion list.FGDN,FCGMM/I,FCWM,WCMPR/IR,WELLSHUT,STORE,AIM,CVTYPE,AMF,PREFT,ZCRITVIS,SPECHA-H). Added as a small hand-maintained supplement merged in by both the extension and the corpus harness.WELLSHUTis list-shaped so it absorbs its well-name records (clearing theINJ1cascade).CGMIRL,CGMPRL, …) emit recognised SUMMARY entries with no shape, so their/-terminated name list (CGMIRL/INJ1 ///) mistook the well name for a keyword. Normalised toarray, with a guard so a column-1 token that is itself a recognised SUMMARY vector by shape (UDQ name, region-set, L-modifier, deck_name_regex family) starts its own vector rather than being swallowed by a bare enable-keyword likePERFORMA.Remaining (7) — deferred, need a decision
column-1: indented but realEQUALS(1) andTUNING(2) in known-good decks. Whether OPM Flow tolerates leading whitespace before keywords is a factual parser question; thecolumn-1lint and its message would be wrong if it does. Not changed on inference.WTEMP/TEMPVDwrong-section +TEMPVDterminator (3): SUMMARY/SOLUTION usages colliding with the SCHEDULE/PROPS index entry — would require overriding generated entries.AQUFETmissing-terminator (1): single edge case vs.AQUFETP.