Store primary variables in well state, and improve well initialization#6114
Store primary variables in well state, and improve well initialization#6114GitPaean merged 7 commits intoOPM:masterfrom
Conversation
|
jenkins build this failure_report please |
| if (well->isProducer() && !zero_target) { | ||
| well->updateWellStateRates(simulator_, this->wellState(), local_deferredLogger); | ||
| //const bool zero_target = well->stoppedOrZeroRateTarget(simulator_, this->wellState(), local_deferredLogger); | ||
| if (well->isProducer()){// && !zero_target) { |
There was a problem hiding this comment.
Here, I think this is the main change of this PR from #5990 . Just putting note for own reference.
| if (stop_or_zero_rate_target && seg == 0) { | ||
| value_[seg][WQTotal] = 0; | ||
| } | ||
| assert(ws.initializedFromReservoir()); |
There was a problem hiding this comment.
It is because of the if (well->isProducer()){// && !zero_target) { above, it caused many assertion failures, I believe. Because injectors also go here.
60d47cd to
d0f70c6
Compare
|
I think this should be ready now. I have addressed the assert() issue pointed out by @GitPaean above. The major changes are:
|
|
jenkins build this failure_report please |
| WellState<Scalar>& well_state, | ||
| DeferredLogger& deferred_logger) const | ||
| { | ||
| assert(isProducer()); |
8a8f761 to
d791cdc
Compare
|
jenkins build this failure_report please |
d791cdc to
13461a4
Compare
|
jenkins build this failure_report please |
d0ad9b6 to
52f07a7
Compare
|
jenkins build this failure_report please |
| const int gas_pos = pu.phase_pos[Gas]; | ||
| value_[seg][GFrac] = well_.scalingFactor(gas_pos) * segment_rates[well_.numPhases() * seg + gas_pos] / total_seg_rate; | ||
| } | ||
| // what about water and gas injection? |
There was a problem hiding this comment.
Just explaining the question in the comments, for other segments, they can use the same formula, for top segments, segment rates are the well rates, for water and gas injection, they will have a non-zero rates for either water or gas, while it does not break the formula either.
There was a problem hiding this comment.
If you agree with the response, you can remove the comment.
| // This also applies to the ORAT/WRAT/GRAT above, but then only the | ||
| // rate not set in the above will be modified in | ||
| // WellInterface::initializeProducerWellState(). | ||
| if (this->pu.phase_used[BlackoilPhases::Liquid]) { |
There was a problem hiding this comment.
I might miss something here, and not talking about the approach either, but from the code level, will we miss something by std::fill(this->surface_rates.begin(), this->surface_rates.end(), 0.); here?
| const double factor = rates_evaluated_at_1bar ? 0.5 : 1.0; | ||
| for (int p = 0; p < this->number_of_phases_; ++p) { | ||
| ws.surface_rates[p] = well_q_s[this->flowPhaseToModelCompIdx(p)]; | ||
| ws.surface_rates[p] = factor * well_q_s[this->flowPhaseToModelCompIdx(p)]; |
There was a problem hiding this comment.
Do you see any situation that the factor matters? The efforts is about to improve the fractions.
|
I checked the regression failures a little bit, not exhaustive. Most of the cases look fine. while the mpi.compareECLFiles_flow+WVFPEXP-02 , mpi.compareECLFiles_flow+MOD4_UDQ_ACTIONX, maybe also can get some checking. |
52f07a7 to
5f4a35a
Compare
|
Thanks to @GitPaean for testing, and pointing out some errors that are now hopefully fixed. |
|
jenkins build this failure_report please |
|
jenkins build this failure_report please |
|
The regression failure looks similar while there are more regressions. I will pick a couple to investigate some to see what is the problem. The one I will look at 02_editnnc, 09_multiply, 15_wvfpexp_02, maybe also the network one. |
|
jenkins build this failure_report please |
|
The number of the regression failures, 191, stays unchanged and let me check the detailed comparisons. https://ci.opm-project.org/job/opm-simulators-PR-builder/8015/#showFailuresLink |
| const int comp_idx_nz = this->flowPhaseToModelCompIdx(nonzero_rate_index); | ||
| if (std::abs(well_q_s[comp_idx_nz]) > floating_point_error_epsilon) { | ||
| const Scalar computed_rate = well_q_s[nonzero_rate_index]; | ||
| if (std::abs(initial_nonzero_rate) < std::abs(computed_rate)) { |
There was a problem hiding this comment.
Why do we need to check std::abs(initial_nonzero_rate) < std::abs(computed_rate)? It might save some small computation potentially, while it adds more complication to the code. The results should be the same?
There was a problem hiding this comment.
If initial_nonzero_rate is greater than computed_rate we should not scale everything to match that rate, because the computed_rate comes from a bhp limit and we should not go above that rate (i.e. factor would be > 1).
An example: we have a GRAT control set to a high number as an important constraint for the later parts of the simulation, but at the start of simulation the gas rate is low. Then we might scale the rate up to be much higher than the BHP-limit rate.
This would be an issue unrelated to this PR then, but something we should figure out and fix. |
| if (well->isProducer() && !zero_target) { | ||
| well->updateWellStateRates(simulator_, this->wellState(), local_deferredLogger); | ||
| if (well->isProducer()) { | ||
| well->initializeProducerWellState(simulator_, this->wellState(), local_deferredLogger); |
There was a problem hiding this comment.
removing && !zero_target caused the regression for the case, it is part of the difficulties that difficult to describe the flow fractions for zero rate situation when we use only surface rates to initialize the primary variables. We should have fraction information, basically zero total rate and proper fraction for the phases.
There was a problem hiding this comment.
not saying the master branch is well designed, I believe the initialized rates play difference in the following logic with nonzero_rate_original.
if (converged) {
const bool zero_target = this->wellUnderZeroRateTarget(simulator, well_state, deferred_logger);
if (this->wellIsStopped() && !zero_target && nonzero_rate_original) {
// Well had non-zero rate, but was stopped during local well-solve. We re-open the well
// for the next global iteration, but if the zero rate persists, it will be stopped.
// This logic is introduced to prevent/ameliorate stopped/revived oscillations
this->operability_status_.resetOperability();
this->openWell();
deferred_logger.debug(" " + this->name() + " is re-opened after being stopped during local solve");
}
}It will open a well previously stopped due to not solvable or operable.
|
jenkins build this failure_report please |
|
- Discard stored primary variables in updateWellStateWithTarget(). - Ensure primary variables are updated to match WellState before solving wells.
|
jenkins build this failure_report please |
|
|
||
| // Discard old primary variables, the new well state | ||
| // may not be anywhere near the old one. | ||
| ws.primaryvar.resize(0); |
There was a problem hiding this comment.
should we just call updatePrimaryVariable at the end of the this function instead so make sure the updated well state will be reflected in the primary variables?
There was a problem hiding this comment.
The problem is that if we do that, the primary variables will read the values stored in ws.primaryvar (which are not modified in this function) instead of setting the primary variables based on surface_rates etc. An alternative would be a boolean flag similar to what was in earlier versions of this, but the current version requires no extra data member so I prefer it.
|
the regressions look good. I think we can update_data now from my side. |
Good. Confirmed that I now get identical results with my previous testing |
|
jenkins build this update_data please |
Reason: PR OPM/opm-simulators#6114 opm-common = dd017b2947babb2f021044d3b04221d744de946a opm-grid = d908ec89a9f83d17a419cb0abfde8e0481f6cb28 opm-simulators = 595b2348dd5c71fc83f2c35d1de9c53418609118 ### Changed Tests ### * spe1flowexp * spe1case1 * spe1case1_import * spe1case2_krnum * spe1case2_thermal * spe1case2_thermal_watvisc * spe1case1_brine * spe1case1_precsalt * gasoil_precsalt * network-01 * network-01_standard * network-01-reroute * network-01-reroute_std * network_01_wtest * spe1_metric_vfp1 * jfunc_01 * ctaquifer_2d_oilwater * numerical_aquifer_3d_2aqu * numerical_aquifer_3d_1aqu * aquflux_01 * spe9 * spe9(restart) * spe9group * spe9group_resv * msw_2d_h * msw_3d_hfa * polymer_injectivity * spe5 * msw_model_1 * base_model_1 * faults_model_1 * base_model2_welpi * 0a1_grpctl_msw_model2 * 0a2_grpctl_msw_model2 * 0a3_grpctl_msw_model2 * 0a4_grpctl_msw_model2 * udq_actionx * udq_wconprod * actionx_m1 * waghyst2 * gpmaint11 * gconprod_t1l * gconprod_t1w * gconprod_t2o * pinch_pinch_multz_all * pinch_pinch_multz-_all * pinch_pinch_multz_all_barrier * pinch_pinch_multz-_all_barrier * pinch_pinch10_nopinch * pinch_t1a_gap * pinch_t1a_nogap * pinch_t1a_nopinch * pinch_t1a1_nogap * pinch_t2a1_gap * pinch_t2a_nopinch * pinch_t2a_gap * pinch_t1b_nopinch * pinch_t1b1_gap * pinch_t1b2_gap * pinch_t1b3_gap * pinch_t1b4_gap * pinch_t1b5_gap * pinch_t1b6_gap * pinch_t1c_nopinch * pinch_t1c1_nogap * pinch_t1c1_gap * pinch_t1c2_gap * pinch_t1c2_nogap * pinch_t1c3_gap * pinch_t1c3_nogap * pinch_t1d_nopinch * pinch_t1d1_gap * pinch_t1d1_nogap * udt-1d-03 * equalreg_multy_1 * equalreg_multy_2 * equalreg_multy_3 * equalreg_multy_4 * equalreg_multy_6 * WCYCLE_0 * WCYCLE_1 * WCYCLE_2 * WCYCLE_3 * WCYCLE_4 * WCYCLE_5 * WCYCLE_6 * WCYCLE_7 * WCYCLE_8 * udq_undefined * udq_in_actionx * reg_smry_in_fld_udq * actionx_gconinje * actionx_gconprod * actionx_udq * actionx_wconhist * actionx_wconinjh * actionx_wefac * udq-01 * cskin_1 * cskin_2 * cskin_3 * cskin_4 * co2store_gaswat * h2store_gaswat * udq_pyaction * 0_base_model2 * 0_base_model2_let * 0a1_grctrl_lrat_orat_base_model2_stw * 0a2_grctrl_lrat_orat_ggr_base_model2_stw * 0a3_grctrl_lrat_lrat_base_model2_stw * 0a4_grctrl_lrat_lrat_ggr_base_model2_stw * 0c_base_fbhpdef * 1_multregt_model2 * 2_multxyz_model2 * 3_a_mpi_multflt_sched_model2 * 5_swatinit_model2 * 6_endscale_model2 * 7_hysteresis_model2 * 8_multiply_tranxyz_model2 * 9_editnnc_model2 * 9_1a_depl_max_rate_min_bhp_stw * 9_1a_depl_max_rate_min_bhp_msw * 9_1b_depl_max_rate_min_thp_stw * 9_1b_depl_max_rate_min_thp_msw * 9_2a_depl_gconprod_1l_stw * 9_2a_depl_gconprod_1l_msw * 9_2b_depl_gconprod_2l_stw * 9_2b_depl_gconprod_2l_msw * 9_3a_ginj_rein-g_stw * 9_3a_ginj_rein-g_msw * 9_3b_ginj_gas_export_stw * 9_3b_ginj_gas_export_msw * 9_3c_ginj_gas_gconsump_stw * 9_3c_ginj_gas_gconsump_msw * 9_3d_ginj_gas_max_export_msw * 9_3e_gas_min_export_stw * 9_3e_gas_min_export_msw * 9_4a_winj_maxwrates_maxbhp_gconprod_1l_stw * 9_4a_winj_maxwrates_maxbhp_gconprod_1l_msw * 9_4b_winj_vrep-w_stw * 9_4b_winj_vrep-w_msw * 9_4c_winj_ginj_vrep-w_rein-g_stw * 9_4c_winj_ginj_vrep-w_rein-g_msw * 9_4d_winj_ginj_gas_export_stw * 9_4d_winj_ginj_gas_export_msw * multflt_model2 * multpvv_model2 * 9_3d_grpctl_stw_model2 * model4_udq_group * model4_gefac * model6_msw * wsegsicd * wsegaicd * wsegvalv * wsegvalv_2d_vert * spe1_foam * spe1_solvent_foam * spe1_gaswater_solvent * norne_reperf * compl_smry * 3d_tran_operator * 0_base_model6 * 0a_aquct_model6 * 0b_rocktab_model6 * base_wt_tracer * base_wt_tracer(restart) * min_bhp_1 * min_bhp_2 * min_bhp_3 * min_thp_1 * max_gor_1 * min_gasrate_1 * min_qoil_1 * max_watercut_1 * max_watercut_2 * max_watercut_3 * max_watercut_4 * rxft_smry * actionx_wpimult * wvfpexp_02 * krnum-03y * krnum-03z * 01_wgrupcon * 02_wgrupcon * winjmult_stdw * winjmult_msw * winjdam_stdw * winjdam_msw * multflt-03 * gsatprod * spe1_float
|
jenkins build this opm-tests=1338 please |
Automatic Reference Data Update for PR OPM/opm-simulators#6114
|
The downstream reference has been installed, I am merging this PR in now. |
















Revised and rebased, based on #5990 originally. Removed some changes to simplify investigation.
For now, just for testing.
Description of the change for the manual: (new edit)
This changes well behaviour in two ways: