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.

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

Fix/actionx compdat #5488

Merged
merged 3 commits into from
Jul 30, 2024
Merged

Fix/actionx compdat #5488

merged 3 commits into from
Jul 30, 2024

Conversation

lisajulia
Copy link
Contributor

@lisajulia lisajulia commented Jul 25, 2024

Fix error when using the keyword COMPDAT in ACTIONX: For parallel runs, the connections opened in ACTIONX are unknown to the grid partitioner, I've added a test comparing two runs (one with actionx and one without) which fails for the current master (#5486) and also a fix which makes this test pass.

This PR depends on OPM/opm-tests#1199, OPM/opm-grid#742 and OPM/opm-common#4150 and neither of these PRs affect the reference manual.

Two data files get compared: one with ACTIONX and one with the same commands but without ACTIONX.
For sequential runs, they produce the same output, but for parallel runs they don't, because the when the COMPDAT keyword is used in an ACTIONX, well connections get created there that are not known to the grid partitioner.
@lisajulia
Copy link
Contributor Author

jenkins build this opm-tests=1199 opm-grid=742 opm-common=4150 please

@lisajulia lisajulia force-pushed the fix/ACTIONX-COMPDAT branch from 2036d04 to 1654e34 Compare July 25, 2024 17:13
@lisajulia
Copy link
Contributor Author

jenkins build this opm-tests=1199 opm-grid=742 opm-common=4150 please

1 similar comment
@lisajulia
Copy link
Contributor Author

jenkins build this opm-tests=1199 opm-grid=742 opm-common=4150 please

Copy link
Member

@blattms blattms left a comment

Choose a reason for hiding this comment

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

Looks good. I have just two suggestions for improvement.

opm/simulators/flow/GenericCpGridVanguard.cpp Outdated Show resolved Hide resolved
opm/simulators/linalg/ISTLSolverBda.cpp Outdated Show resolved Hide resolved
@blattms
Copy link
Member

blattms commented Jul 29, 2024

Once this is fixed, please trigger a test in the opm-common PR. That will also run the tests there.

@lisajulia lisajulia force-pushed the fix/ACTIONX-COMPDAT branch from 1654e34 to 8a261f0 Compare July 29, 2024 07:56
@lisajulia lisajulia force-pushed the fix/ACTIONX-COMPDAT branch from 8a261f0 to efb18d0 Compare July 29, 2024 08:00
@lisajulia lisajulia requested a review from blattms July 29, 2024 08:18
@blattms
Copy link
Member

blattms commented Jul 29, 2024

According to @atgeirr we also need to take the future connections into account in BlackoilModelNldd.hpp#L944 and files opm/simulators/utils/ParallelNLDDPartitioningZoltan.*pp

@lisajulia
Copy link
Contributor Author

lisajulia commented Jul 29, 2024

According to @atgeirr we also need to take the future connections into account in BlackoilModelNldd.hpp#L944 and files opm/simulators/utils/ParallelNLDDPartitioningZoltan.*pp

Yes, thanks, I can take care of that, but can we still merge this one and OPM/opm-tests#1199, OPM/opm-grid#742 and OPM/opm-common#4150 already? I had a brief look and I think the same approach works for NLDD and I would then open a new PR for that.

@atgeirr
Copy link
Member

atgeirr commented Jul 29, 2024

I had a brief look and I think the same approach words for NLDD and I would then open a new PR for that.

That sounds good! It is fine to do this in two steps.

@blattms blattms mentioned this pull request Jul 30, 2024
@blattms
Copy link
Member

blattms commented Jul 30, 2024

This definitely makes #5494 disappear. Tested on my system.

Hence, many thanks for finding and fixing this rather severe problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants