Skip to content

Test file deck iteration over empty blocks - #5334

Merged
bska merged 1 commit into
OPM:masterfrom
CeetronSolutions:filedeck-empty-blocks
Sep 9, 2026
Merged

Test file deck iteration over empty blocks#5334
bska merged 1 commit into
OPM:masterfrom
CeetronSolutions:filedeck-empty-blocks

Conversation

@kriben

@kriben kriben commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

FileDeck::Index steps through the keywords of one input file at a time, and erasing every keyword of an included file leaves a block with no keywords behind. operator++ then stopped on that block and compared the keyword index to blocks[file_index].size() - 1, which wraps around for an empty block, so the iteration never left the block. operator-- placed the index on the same wrapped keyword index, and dereferencing it threw.

Move past empty blocks in both operators. Reverse iteration reports the same error as an index at the start of the deck if only empty blocks remain.

@bska

bska commented Sep 7, 2026

Copy link
Copy Markdown
Member

PR #5341, which was recently merged into the master branch, addressed a similar problem. Would you mind checking if this problem still persist in the latest development sources?

@bska bska added the manual:irrelevant This PR is a minor fix and should not appear in the manual label Sep 7, 2026
@kriben
kriben force-pushed the filedeck-empty-blocks branch from 7dc6be8 to a8f5cdb Compare September 9, 2026 09:46
@kriben kriben changed the title Skip empty file blocks when iterating a FileDeck Test file deck iteration over empty blocks Sep 9, 2026
@kriben

kriben commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

PR #5341, which was recently merged into the master branch, addressed a similar problem. Would you mind checking if this problem still persist in the latest development sources?

@bska Yes, PR #5341 fixed the same issue. I have updated my PR so that only the unit test remains. The unit test explicitly tests backward iteration which was not tested well in the other PR.

@bska

bska commented Sep 9, 2026

Copy link
Copy Markdown
Member

jenkins build this please

@bska

bska commented Sep 9, 2026

Copy link
Copy Markdown
Member

Looks like we may have a stale commit graph on GitHub which causes (erroneous) regression test failures. I'll temporarily close this PR now and reopen it in a few minutes to see if that's sufficient to get GitHub to agree on what the commit graph is...

@bska bska closed this Sep 9, 2026
@bska bska reopened this Sep 9, 2026
@bska

bska commented Sep 9, 2026

Copy link
Copy Markdown
Member

jenkins build this please

@bska bska closed this Sep 9, 2026
@bska bska reopened this Sep 9, 2026
@bska

bska commented Sep 9, 2026

Copy link
Copy Markdown
Member

jenkins build this please

@bska

bska commented Sep 9, 2026

Copy link
Copy Markdown
Member

jenkins build this update_data please

@bska

bska commented Sep 9, 2026

Copy link
Copy Markdown
Member

jenkins build this opm-tests=1596 please

bska pushed a commit to OPM/opm-tests that referenced this pull request Sep 9, 2026
Reason: PR OPM/opm-common#5334

opm-common     = a732b077f3960c6101c23e5927b86ff94f42c9b2
opm-grid       = 798166c5de06f16cc4d9d0acd19d15cc681f6ffd
opm-simulators = 99920922be1d7ae5b90fa6a36cb3033d20899856

### Changed Tests ###

  * fpr_nonhc
  * micp
  * gasoil_precsalt
  * sshift_compositional
bska added a commit to OPM/opm-tests that referenced this pull request Sep 9, 2026

@bska bska left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for the updates. This looks good to me now and I'll merge into master.

Apologies for the many confusing build triggers. That turned out to be an unrelated issue.

@bska
bska merged commit 2e3a566 into OPM:master Sep 9, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

manual:irrelevant This PR is a minor fix and should not appear in the manual

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants