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

Remove functions to filter connections #1059

Closed

Conversation

joakim-hove
Copy link
Member

@joakim-hove joakim-hove commented Oct 1, 2019

It is all about active cells!

Background

The first implementation of the EclipseGrid in opm-common only took the ACTNUM values from the deck into account. Then at a later stage the grid processing code would calculate PORV and possibly deactivate some additional cells. When assembling the WellConnectionsobject the cells will be checked, and connections to inactive cells will be ignored. However, because the downstream grid processing might deactivate addition cells we must check the connections in a second pass. This is the purpose of the Schedule::filterConnections() method.

Since the original implementation of the EclipseGrid the PORV calculations have been moved to opm-common, we should therefor now be in a situation where the ACTNUM in the original EclipseGrid is complete, and no further cells should be deactivated. This PR removes the filterConnections() functionality and downstream PR's remove the calls to filterConnections(). This will be one step further to get an immutable Schedule object, and should also help in currently ongoing parallellizing efforts.

Disclaimer
The grid processing code is complex, and not very well tested in terms of unit tests. I might have misunderstood things, this set of PR's should be tested on relevant industrial models beyond the testcases found in jenkins.

OPM/opm-upscaling#279
OPM/opm-simulators#2041
OPM/opm-grid#393

@blattms
Copy link
Member

blattms commented Oct 2, 2019

Thanks.

There was an email conversation between @joakim-hove, @GitPaean and me about that functionality.
AFAIK part of the result was:

  • Multisegment wells rely on all the connections being valid (i.e. to active cells). Traditional wells check another time. Hence filtering is needed
  • Filtering is always done on the global grid.
  • The filtered connections are used for the initial output writing. (I need to double check here but that is my feeling)

Not sure whether the last point isn't rather a bug than a feature.

@joakim-hove
Copy link
Member Author

jenkins build this opm-grid=393 opm-simulators=2041 please

@joakim-hove
Copy link
Member Author

jenkins build this opm-grid=393 opm-simulators=2041 please

@joakim-hove joakim-hove changed the title WIP: Remove functions to filter connections Remove functions to filter connections Oct 4, 2019
@joakim-hove
Copy link
Member Author

jenkins build this opm-grid=393 opm-simulators=2041 opm-upscaling=279 please

1 similar comment
@joakim-hove
Copy link
Member Author

jenkins build this opm-grid=393 opm-simulators=2041 opm-upscaling=279 please

@GitPaean
Copy link
Member

GitPaean commented Oct 4, 2019

for one real case, it throws at the WellsManager_impl.hpp:163

153                     const int i = completion.getI();
154                     const int j = completion.getJ();
155                     const int k = completion.getK();
156 
157                     const int* cpgdim = cart_dims;
158                     const int cart_grid_indx = i + cpgdim[0]*(j + cpgdim[1]*k);
159                     const std::map<int, int>::const_iterator cgit = cartesian_to_compressed.find(cart_grid_indx);
160                     if (cgit == cartesian_to_compressed.end()) {
161                         const std::string msg = ("Cell with i,j,k indices " + std::to_string(i) + " " + std::to_string(j)
162                                     + " " + std::to_string(k)  +  " not found in grid (well = " +  well.name() + ").");
163                         OPM_THROW(std::runtime_error, msg);
164                     }

@joakim-hove
Copy link
Member Author

for one real case, it throws at the WellsManager_impl.hpp:163

Thank you for looking into this - that is extremely valuable. I must say I was a little surprised by where the exception was raised; it "should" have been here, anyway I will look into it.

@bska
Copy link
Member

bska commented Sep 27, 2020

@joakim-hove : Are we going to pick this up or should we just close this PR?

@bska
Copy link
Member

bska commented May 20, 2022

I'm closing this now. The patch no longer applies cleanly and nobody appears to care enough to shepherd it into master.

@bska bska closed this May 20, 2022
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.

None yet

4 participants