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

Actually respect NUPCOL (not NUPCOL+1) and allow network to converge with GLO #6100

Merged
merged 1 commit into from
Apr 2, 2025

Conversation

vkip
Copy link
Member

@vkip vkip commented Mar 21, 2025

Also:

  • Some more informative output in case network does not converge
  • Change order of return arguments in updateWellControlsAndNetworkIteration to be consistent with updateWellControls

@vkip
Copy link
Member Author

vkip commented Mar 21, 2025

jenkins build this failure_report please

@totto82
Copy link
Member

totto82 commented Mar 21, 2025

Note. I have a PR where this code is refactored. #6095
The refactoring also contains more info if the network does not converge.

Copy link
Member

@totto82 totto82 left a comment

Choose a reason for hiding this comment

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

Thank you for the contribution. I just have one minor question.

@@ -1743,7 +1751,7 @@ namespace Opm {


template<typename TypeTag>
std::pair<bool, bool>
std::tuple<bool, bool, typename BlackoilWellModel<TypeTag>::Scalar>
BlackoilWellModel<TypeTag>::
Copy link
Member

Choose a reason for hiding this comment

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

Cant you just use Scalar here?

Copy link
Member Author

Choose a reason for hiding this comment

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

I believe I tried that first, and it didn't compile at my end (GCC13)

Copy link
Member

Choose a reason for hiding this comment

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

Scalar has no meaning in the context as the return value is defined "outside" the class context. typename is required.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks. I see that now. @vkip can you undraft this if you think it is ready for merging.

@totto82
Copy link
Member

totto82 commented Apr 2, 2025

jenkins build this failure_report please

@vkip vkip marked this pull request as ready for review April 2, 2025 07:35
@vkip
Copy link
Member Author

vkip commented Apr 2, 2025

Test failures are mainly due to different time-stepping. I've been through all the failed tests with shorter time-steps and find results to be (visually) identical to master, except for two tests that differ in a single time-step - these become identical to master when using NUPCOL=11.

@totto82
Copy link
Member

totto82 commented Apr 2, 2025

Thanks for looking into the test failures. I will start the process of updating the reference results and merging this.

@totto82
Copy link
Member

totto82 commented Apr 2, 2025

jenkins build this update_data please

jenkins4opm pushed a commit to jenkins4opm/opm-tests that referenced this pull request Apr 2, 2025
Reason: PR OPM/opm-simulators#6100

opm-common     = 034b510205f219335dd01f8b7770990102df6b12
opm-grid       = 80c23280f8eb5ab7b6ad955c6c400b19e041f663
opm-simulators = a818aaf168a3ef196eeff0ec90567814062576ab

### Changed Tests ###

  * udq_wconprod
  * gconprod_t1l
  * udq_pyaction
  * 9_3a_grpctl_msw_model2
  * 9_3b_grpctl_msw_model2
  * 9_3d_grpctl_stw_model2
  * 9_3d_grpctl_msw_model2
  * 9_4a_grpctl_stw_model2
  * 9_4a_grpctl_msw_model2
  * 9_4b_grpctl_msw_model2
  * wvfpexp_02
  * krnum_02z
  * network_01_wtest
@totto82
Copy link
Member

totto82 commented Apr 2, 2025

jenkins build this opm-tests=1322 please

totto82 added a commit to OPM/opm-tests that referenced this pull request Apr 2, 2025
@totto82 totto82 merged commit 27e5555 into OPM:master Apr 2, 2025
1 check passed
@vkip vkip deleted the nupcol3_and_nwglo branch April 2, 2025 20:02
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