Skip to content

remove the functions related to Schedule::filterConnections()#4975

Merged
bska merged 2 commits into
OPM:masterfrom
GitPaean:remove_filter-connections
Feb 10, 2026
Merged

remove the functions related to Schedule::filterConnections()#4975
bska merged 2 commits into
OPM:masterfrom
GitPaean:remove_filter-connections

Conversation

@GitPaean

@GitPaean GitPaean commented Feb 5, 2026

Copy link
Copy Markdown
Member

was trying to rebase #1059 , while it turns out be more work than just making a new one.

The PR is following the discussion in OPM/opm-simulators#6787 .

@GitPaean GitPaean added the manual:irrelevant This PR is a minor fix and should not appear in the manual label Feb 5, 2026
@GitPaean

GitPaean commented Feb 5, 2026

Copy link
Copy Markdown
Member Author

jenkins build this opm-simulators=6806 please

@GitPaean GitPaean force-pushed the remove_filter-connections branch from 8ae4621 to 8436a83 Compare February 5, 2026 13:34
@GitPaean

GitPaean commented Feb 5, 2026

Copy link
Copy Markdown
Member Author

jenkins build this opm-simulators=6806 please

@GitPaean

GitPaean commented Feb 5, 2026

Copy link
Copy Markdown
Member Author

the function

void filter(const ActiveGridCells& grid);
is only used by the test
BOOST_AUTO_TEST_CASE(ApplyWellPI) {

Should we keep the test or modify the test? I believe it is a question for @bska

@bska

bska commented Feb 5, 2026

Copy link
Copy Markdown
Member

Should we keep the test or modify the test? I believe it is a question for @bska

I need to think about it, but my first hunch is to adapt the test function to the new reality of filterConnections() not existing.

@GitPaean GitPaean force-pushed the remove_filter-connections branch from 8436a83 to cb4438c Compare February 6, 2026 22:31
@GitPaean

GitPaean commented Feb 6, 2026

Copy link
Copy Markdown
Member Author

Should we keep the test or modify the test? I believe it is a question for @bska

I need to think about it, but my first hunch is to adapt the test function to the new reality of filterConnections() not existing.

I updated the test without using the filter() function and remove the function WellConnections::filter() .

I thought for some, it is also okay to keep the function and not change the test, so I make this change in a separate commit, so it is easier to revert or modify.

Now I consider the PR is ready for review.

@GitPaean GitPaean marked this pull request as ready for review February 6, 2026 22:33
@GitPaean

GitPaean commented Feb 6, 2026

Copy link
Copy Markdown
Member Author

jenkins build this opm-simulators=6806 please

@bska bska left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I updated the test without using the filter() function and remove [that function]. I thought for some, it is also okay to keep the function and not change the test, so I make this change in a separate commit, so it is easier to revert or modify.

I prefer the current version. The filter() operation really doesn't make any sense when we have event driven actions that will just undo whatever filtering we'd applied previously. Essentially, the filter() operation assumes that the Schedule object is unchanging once constructed and that assumption doesn't hold anymore when we have actions.

I'll merge this and its downstream counterpart into the master branch.

@bska bska merged commit 92810e7 into OPM:master Feb 10, 2026
2 checks passed
@GitPaean GitPaean deleted the remove_filter-connections branch February 10, 2026 21:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

manual:irrelevant This PR is a minor fix and should not appear in the manual

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants