Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Write MULTPV to INIT-file and fix a few minor issues with multiple MULTPV occurrences #4015

Merged
merged 3 commits into from
Apr 18, 2024
Merged

Write MULTPV to INIT-file and fix a few minor issues with multiple MULTPV occurrences #4015

merged 3 commits into from
Apr 18, 2024

Conversation

blattms
Copy link
Member

@blattms blattms commented Apr 16, 2024 •
edited

This output was missing from the init file previously. Now MULTPV will be output if it is specified in the GRID or EDIT section.

Support for MULTPV in the SCHEDULE section will be added in an upcoming PR. I need to sort out the parallel handling of that.

@blattms blattms changed the title Write MULTPV to INIT-file and fix a few minor issues with multiple MULTPV occurences Write MULTPV to INIT-file and fix a few minor issues with multiple MULTPV occurrences Apr 16, 2024
@blattms
Copy link
Member Author

blattms commented Apr 16, 2024

jenkins build this please

bska
bska approved these changes Apr 16, 2024
View reviewed changes
Copy link
Member

@bska bska left a comment

Choose a reason for hiding this comment

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

Very good. We should merge this when we're satisfied that we don't introduce any problems for the simulations. I just have one question regarding a slightly different way of writing the unit test so as to exercise more of the parser.

tests/test_EclipseIO.cpp Outdated Show resolved Hide resolved
@blattms
Copy link
Member Author

blattms commented Apr 16, 2024

jenkins build this please

@blattms
Copy link
Member Author

blattms commented Apr 16, 2024

I will need to double check the failing FieldPropsTests Maybe my idea of treating MULTPV just as other multipliers is wrong.

@blattms blattms marked this pull request as draft April 16, 2024 14:29
@blattms
Copy link
Member Author

blattms commented Apr 16, 2024

Seems like MULTPV in the EDIT section is rather special. It should only work on values that have been edited there. (Not even sure what that means.). Didn't see that coming... Not sure whether master does this correct.

@blattms
Copy link
Member Author

blattms commented Apr 17, 2024

jenkins build this please

@blattms blattms force-pushed the feature/write-multpv-to-init branch 2 times, most recently from f5c32ea to 3a65887 Compare April 17, 2024 13:31
@blattms
Copy link
Member Author

blattms commented Apr 17, 2024

jenkins build this please

@blattms
Copy link
Member Author

blattms commented Apr 17, 2024

jenkins build this ignore_extra please

@blattms blattms marked this pull request as ready for review April 18, 2024 06:09
@blattms
Copy link
Member Author

blattms commented Apr 18, 2024

jenkins build this ignore_extra please

@blattms blattms added this to the Release 2024.04 milestone Apr 18, 2024
@blattms blattms force-pushed the feature/write-multpv-to-init branch from 3a65887 to 0bf3f13 Compare April 18, 2024 08:43
@blattms
Copy link
Member Author

blattms commented Apr 18, 2024

I have removed my so-called fix as it broke the PORV of MULTREGT-01.

@blattms blattms force-pushed the feature/write-multpv-to-init branch from 0bf3f13 to 265571d Compare April 18, 2024 08:45
@blattms
Copy link
Member Author

blattms commented Apr 18, 2024

jenkins build this ignore_extra please

@blattms blattms merged commit 489ea20 into OPM:master Apr 18, 2024
@blattms blattms mentioned this pull request Apr 18, 2024
Merged
@blattms
Copy link
Member Author

blattms commented Apr 18, 2024 •
edited

😳 No idea how I managed to push merge here. That was unintended. Really sorry. I have reverted it instantly.

@akva2
Copy link
Member

akva2 commented Apr 18, 2024

whatever you're drinking, i want some

blattms added a commit to blattms/opm-common that referenced this pull request Apr 18, 2024
@blattms
Write MULTPV to INIT-file
8dcae7f 
This output was missing from the init file previously. Now MULTPV will
be output if it is specified in the GRID or EDIT section.  Support for

MULTPV in the SCHEDULE section will be added in an upcoming PR. I need
to sort out the parallel handling of that.

This is is OPM#4015 again
@blattms blattms mentioned this pull request Apr 18, 2024
Merged
blattms added a commit to blattms/opm-common that referenced this pull request Apr 22, 2024
@blattms
Write MULTPV to INIT-file and fix a few minor issues (v2.0)
3f79508 
This reverts commit a0e01b4 which
reverted my initial try in OPM#4015.

The problem in said PR was that one of the tests (MULTREGT-01) broke
(probably due to applying MULTPV twice).
blattms added a commit to blattms/opm-common that referenced this pull request Apr 30, 2024
@blattms
Write MULTPV to INIT-file and fix a few minor issues (v2.0)
9ba5515 
This reverts commit a0e01b4 which
reverted my initial try in OPM#4015.

The problem in said PR was that one of the tests (MULTREGT-01) broke
(probably due to applying MULTPV twice).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bska bska bska approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
Release 2024.04
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@blattms @akva2 @bska