{"action":"created","issue":{"url":"https://api.github.com/repos/OPM/opm-simulators/issues/1118","repository_url":"https://api.github.com/repos/OPM/opm-simulators","labels_url":"https://api.github.com/repos/OPM/opm-simulators/issues/1118/labels{/name}","comments_url":"https://api.github.com/repos/OPM/opm-simulators/issues/1118/comments","events_url":"https://api.github.com/repos/OPM/opm-simulators/issues/1118/events","html_url":"https://github.com/OPM/opm-simulators/pull/1118","id":219205114,"number":1118,"title":"[WIP] Changes to improve the StandardWellsDense","user":{"login":"GitPaean","id":6847941,"avatar_url":"https://avatars3.githubusercontent.com/u/6847941?v=3","gravatar_id":"","url":"https://api.github.com/users/GitPaean","html_url":"https://github.com/GitPaean","followers_url":"https://api.github.com/users/GitPaean/followers","following_url":"https://api.github.com/users/GitPaean/following{/other_user}","gists_url":"https://api.github.com/users/GitPaean/gists{/gist_id}","starred_url":"https://api.github.com/users/GitPaean/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/GitPaean/subscriptions","organizations_url":"https://api.github.com/users/GitPaean/orgs","repos_url":"https://api.github.com/users/GitPaean/repos","events_url":"https://api.github.com/users/GitPaean/events{/privacy}","received_events_url":"https://api.github.com/users/GitPaean/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":29,"created_at":"2017-04-04T10:25:19Z","updated_at":"2017-04-11T13:09:48Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/OPM/opm-simulators/pulls/1118","html_url":"https://github.com/OPM/opm-simulators/pull/1118","diff_url":"https://github.com/OPM/opm-simulators/pull/1118.diff","patch_url":"https://github.com/OPM/opm-simulators/pull/1118.patch"},"body":"The PR includes mostly the following revisions\r\n\r\n1. Moving the calculation of well potentials at the beginning of each `time step` instead of the end of previous `report step`, which is the better and more correct thing to do.\r\n\r\n    In the old way, we always use the well potentials from the end of the last `report step`. For many cases, it will be fine. The well potentials will be invalid when a new well is newly added, since we do not have this well yet in the last report step. \r\nAnd also, there are some decks, the size of the report step is pretty big, half year / 1 year. For some period, the well potentials will change significantly during the report step, updating the well potentials during the report step is also more desirable.  \r\nWhen THP is involved, the well potentials are closely related to the well rates and also the composition of the rates (water cut, GOR, and so on). Updating the well potential during the report step is also more desirable.\r\n\r\nTODO: computeRESV() should probably be done in `time step` basis too, while it will be interfering with the current way to handle RESV controls, and will be done after the addressing OPM/opm-parser#1032 .\r\n\r\n2. Better way to initialize / update WellState with the well controls. Based on the old composition of the rates, we update the rates for multi-phase targets.  The old implementation is that we only update single phase target. \r\n\r\nTODO: We should never update WellState based on the defaulted BHP value, which will gives a disastrous rates. We need some support from the parser side to achieve this. \r\n\r\n3. Not copying the old control index, always using the the control mode the deck asks for.\r\n\r\n    The reason is that when the control sets changed due to new well control keywords enter, the same control index can mean rather random control mode. It only cause problem when new control keyword is entered, which will result in a random control mode. Most of the time, due to the fully implicit nature of our implementation, a sensible result is likely to be obtained eventually. While when THP control is involved, a random control will result in a non-desirable region in VFP table which will make the simulation fail immediately.  \r\n\r\n    The old way make senses when no new control keyword is entered. As a result, the current PR might slow down the simulation for some / many cases a little bit.  \r\n\r\nTODO: A better way is that for each well, if there is some control keywords enters, we should use the control index that deck ask for, otherwise, it should be pretty safe to use the old control index, which is potentially a better / safe initial guess for the time step. To achieve this, we need some support from the opm-parser side. \r\n\r\n4. An iterative way to calculate the well potentials is introduced when the THP control is involved.  \r\n    Due to the coupling between the rates (potentials are rates) and BHP through VFP table when THP control is involved, we have to calculate the well potentials iteratively to find an consistent matching between potentials and BHP.  It raises problems when a well is newly added. We did not have a reasonable initial guess for the iteration process. In this PR, we calculate a initial value assuming small rates will always be in safe regions in the VFP table.\r\n\r\nTODO: We need more knowledge from the VFP tables to do all the VFP related things better, including the keyword WVFPEXP, which detect the unstable region in VFP so that we can overcome difficulties in THP control related calculation. \r\n\r\n5. Considering the injection phase of the INJECTORS. \r\n   The comp_frac from the `Wells` struct only show the preferred phase of the injection wells, it has nothing to do with the actual injection phase. Using comp_frac in most place in well control related is wrong. This PR makes the `distr` in `well_controls` recognize the injection phase and make use of it. \r\n\r\n\r\nThere are quite a few other changes following the above several relatively major ones.\r\n\r\n1.  Not copying the wellSolutions() from the old well state. The reason is that the content of the first element of wellSolutions() depends on the control type, copying the old values is dangerous when control type changes. It is the culprit of the many of `too large residual for water phase`. This makes this PR cover the PR OPM/opm-simulators#1086\r\n\r\n2. Better treatment of the THP control.  When no THP constraint is involved, we do not calculate the THP related at all, and the thp value will be zero. \r\n\r\n3. When the wells are all under individual control, their group targets will be updated for each iteration.  \r\n\r\nThere are many changes in this PR will affect all the simulations, should be tested thoroughly. And the group control of the legacy `flow` is broken. Will update `flow` after the review of `flow_ebos` is done. \r\n\r\nThis PR requires the PR OPM/opm-core#1152 . \r\n"},"comment":{"url":"https://api.github.com/repos/OPM/opm-simulators/issues/comments/293255832","html_url":"https://github.com/OPM/opm-simulators/pull/1118#issuecomment-293255832","issue_url":"https://api.github.com/repos/OPM/opm-simulators/issues/1118","id":293255832,"user":{"login":"atgeirr","id":1876621,"avatar_url":"https://avatars3.githubusercontent.com/u/1876621?v=3","gravatar_id":"","url":"https://api.github.com/users/atgeirr","html_url":"https://github.com/atgeirr","followers_url":"https://api.github.com/users/atgeirr/followers","following_url":"https://api.github.com/users/atgeirr/following{/other_user}","gists_url":"https://api.github.com/users/atgeirr/gists{/gist_id}","starred_url":"https://api.github.com/users/atgeirr/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/atgeirr/subscriptions","organizations_url":"https://api.github.com/users/atgeirr/orgs","repos_url":"https://api.github.com/users/atgeirr/repos","events_url":"https://api.github.com/users/atgeirr/events{/privacy}","received_events_url":"https://api.github.com/users/atgeirr/received_events","type":"User","site_admin":false},"created_at":"2017-04-11T13:09:48Z","updated_at":"2017-04-11T13:09:48Z","body":"jenkins build this opm-core=1152 opm-data=205 please"}}
{"actor":{"display_login":"atgeirr"}}