Skip to content

Return group/well potentials from GuideRate object #4567

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

hakonhagland
Copy link
Contributor

@hakonhagland hakonhagland commented Apr 11, 2025

Add a helper method to return group/well potentials. This is to be used by the reservoir coupling code in opm-simulators, see OPM/opm-simulators#6170

@hakonhagland
Copy link
Contributor Author

jenkins build this please

@hakonhagland
Copy link
Contributor Author

jenkins build this please

@hakonhagland
Copy link
Contributor Author

jenkins build this please

@hakonhagland hakonhagland requested a review from Copilot April 11, 2025 08:19
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

@hakonhagland hakonhagland requested a review from Copilot April 11, 2025 08:24
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

@hakonhagland hakonhagland requested a review from blattms April 11, 2025 08:28
@hakonhagland
Copy link
Contributor Author

jenkins build this please

@hakonhagland
Copy link
Contributor Author

jenkins build this opm-simulators=6170 please

hakonhagland and others added 3 commits April 11, 2025 13:07
Add a helper method to return group/well potentials. This is to be used
by the reservoir coupling code in opm-simulators.
@hakonhagland
Copy link
Contributor Author

jenkins build this opm-simulators=6170 please

1 similar comment
@hakonhagland
Copy link
Contributor Author

jenkins build this opm-simulators=6170 please

Comment on lines +138 to +143
if (!this->hasPotentials(name)) {
auto message = fmt::format("Potentials for '{}' do not exist.", name);
throw std::logic_error {message};
}
return this->potentials.at(name);
}
Copy link
Member

Choose a reason for hiding this comment

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

You are triggering two searches here: one in hasPotentials and the other one ins at. In addition there is double error handling as at does another check.

If you want the better error message then we could do:

Suggested change
if (!this->hasPotentials(name)) {
auto message = fmt::format("Potentials for '{}' do not exist.", name);
throw std::logic_error {message};
}
return this->potentials.at(name);
}
if (const auto candidate = this->potentials.find(name); candidate == this->potentials.end()) {
auto message = fmt::format("Potentials for '{}' do not exist.", name);
throw std::logic_error {message};
} else {
return candidate->second;
}
}

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.

The failing test mpi.compareECLFiles_flow+MOD4_UDQ_ACTIONX is strange.

At least this PR seems innocent.

@blattms
Copy link
Member

blattms commented Apr 16, 2025

jenkins build this opm-simulators=6170 serial please

@blattms
Copy link
Member

blattms commented Apr 16, 2025

It seems like compilation in serial is broken for the opm-simulator part.

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.

2 participants