introducing GroupTarget struct for SingleWellState #6654

GitPaean · 2025-12-03T13:20:35Z

PRs #6596 and #6545 call for more information through the SingleWellState::group_target.

Introducing a dedicated struct can be one way to make it easier to extend.

In its current form, we are not using the extension for now. It will be potentially used by other PRs like #6596 and #6545 , or debugging purposes.

not sure it is strong argument to get this PR in with its current form.

GitPaean · 2025-12-03T13:21:07Z

jenkins build this please

GitPaean · 2025-12-03T14:06:19Z

could not reproduce the failure.

mpi.compareECLFiles_flow+02-WGRUPCON

will try again.

GitPaean · 2025-12-03T14:06:37Z

jenkins build this failure_report please

GitPaean · 2025-12-03T14:58:43Z

Locally, the PR did not introduce residual difference for the case 02_WGRUPCON.

Somehow it looks there is small difference from this PR while it not big enough to be visible.

steink · 2025-12-03T15:16:00Z

Locally, the PR did not introduce residual difference for the case 02_WGRUPCON.

Somehow it looks there is small difference from this PR while it not big enough to be visible.

Very strange, you haven't introduced anything here that should change the results?

GitPaean · 2025-12-04T00:16:00Z

Locally, the PR did not introduce residual difference for the case 02_WGRUPCON.
Somehow it looks there is small difference from this PR while it not big enough to be visible.

Very strange, you haven't introduced anything here that should change the results?

One thing that this pull request changes is that the comparison operator for the SingleWellState. There are more comparison or stricter criterion than the master branch.

        bool operator==(const GroupTarget& other) const {
            return   ( group_name == other.group_name
                    && target_value == other.target_value
                    && production_cmode == other.production_cmode
                    && injection_cmode == other.injection_cmode );
        }

GitPaean · 2025-12-04T09:01:15Z

jenkins build this failure_report please

GitPaean · 2025-12-04T09:33:51Z

Locally, the PR did not introduce residual difference for the case 02_WGRUPCON.
Somehow it looks there is small difference from this PR while it not big enough to be visible.

Very strange, you haven't introduced anything here that should change the results?

One thing that this pull request changes is that the comparison operator for the SingleWellState. There are more comparison or stricter criterion than the master branch.
        bool operator==(const GroupTarget& other) const {
            return   ( group_name == other.group_name
                    && target_value == other.target_value
                    && production_cmode == other.production_cmode
                    && injection_cmode == other.injection_cmode );
        }

The new test results show it might not be due to this.

GitPaean · 2025-12-04T10:17:59Z

jenkins build this failure_report please

GitPaean · 2025-12-04T10:49:39Z

removing other members from GroupTarget recover the regression failure. Mostly likely it is the std::string member matters. Will test. Could not reproduce it locally, which makes it harder to investigate.

GitPaean · 2025-12-04T10:54:11Z

jenkins build this failure_report please

GitPaean · 2025-12-04T11:57:29Z

jenkins build this please

GitPaean · 2025-12-04T12:16:31Z

jenkins build this please

GitPaean · 2025-12-04T12:41:40Z

jenkins build this please

GitPaean · 2025-12-04T13:01:33Z

9a764f2

recovered the regression. putting here for record.

         static GroupTarget injectionGroupTarget(const std::string& gname,
                                                 Group::InjectionCMode cmode,
                                                 Scalar value) {
+            std::cout << "Creating injection group target for group " << gname
+                      << " with cmode " << Group::InjectionCMode2String(cmode)
+                      << " (Int: " << static_cast<int>(cmode) << ")"
+                      << " and value " << value << std::endl;
             return {gname, Group::ProductionCMode::NONE, cmode, value};
         }
 
         static GroupTarget productionGroupTarget(const std::string& gname,
                                                  Group::ProductionCMode cmode,
                                                  Scalar value) {
+            std::cout << "Creating produduction group target for group " << gname
+                      << " with cmode " << Group::ProductionCMode2String(cmode)
+                      << " (Int: " << static_cast<int>(cmode) << ")"
+                      << " and value " << value << std::endl;
             return {gname, cmode, Group::InjectionCMode::NONE, value};
         }

GitPaean · 2025-12-05T08:44:01Z

jenkins build this please

GitPaean · 2025-12-07T21:09:58Z

jenkins build this please

GitPaean · 2025-12-07T21:42:37Z

cff6970 fixes the regression.

It appears that using -> or (*) for a std::optional will not trigger an exception if the std::optional is std::nullopt.

it might be safer to use .value().

GitPaean · 2025-12-09T12:12:36Z

jenkins build this please

steink

To me it would be good to get this in, since I need a slightly extended version of it in an upcomping PR. So since this part is just a refactorization, I am very much in favour of merging.

bska

I think this is good for the most part, but I'd like some additional information–and ideally some Doxygen-style documentation–about the data members of the new type.

bska · 2025-12-11T12:01:45Z

opm/simulators/wells/SingleWellState.hpp

    };

+    struct GroupTarget {
+        std::string group_name;


The group_name member appears to be unused. Is that intentional?

Basically, nothing in this PR is used in the master branch. It is more about helping other development. Maybe it should go in with other development that actually needs the content in this PR? Please just suggest.

bska · 2025-12-11T12:06:06Z

opm/simulators/wells/SingleWellState.hpp

+    struct GroupTarget {
+        std::string group_name;
+        Group::ProductionCMode production_cmode {Group::ProductionCMode::NONE};
+        Group::InjectionCMode  injection_cmode  {Group::InjectionCMode::NONE};


Might you need one injection control mode for each phase, or at least a way to associate this "target" to a specific injection phase?

In the current form of #6545 , which focuses on production control mode mostly. The injection control mode is not of much use, while your point is valid. I will discuss it with @steink

bska · 2025-12-11T12:06:41Z

opm/simulators/wells/SingleWellState.hpp

+        template<class Serializer>
+        void serializeOp(Serializer& serializer) {
+            serializer(group_name);
+            serializer(production_cmode);
+            serializer(injection_cmode);
+            serializer(target_value);
+        }


Why do you need this function?

not totally sure, actually, i thought it is standard.

what types of data structure need its own serializeOp function? or your question is rather about something else? Thanks.

what types of data structure need its own serializeOp function?

Mostly those that use object serialisation for broadcasting from the MPI I/O rank to other MPI ranks. This function is the underpinning of the object distribution process from loading the model from a .DATA file to starting the actual simulation in parallel but as long as there's no type of MPI communication for the type then we don't really need a serializeOp() member function.

PR #5338 muddies the water a little bit as it uses the object serialisation protocol to collect data onto the I/O rank instead of distributing from the I/O rank, but as I said for the most part we don't really need per-type serializeOp() functions in opm-simulators.

When @steink compiles on his computer, it looks like this part might be needed, possible related to HDF5 support.

so it can record the group name that results the group target, the group control mode (either injecting or producing) of the group, and also the target value enforced to this well.

GitPaean · 2025-12-15T08:36:31Z

I suggest this PR go in with other development that actually uses it, so it is easier to evaluate.

GitPaean · 2025-12-15T11:20:39Z

jenkins build this please

GitPaean · 2025-12-15T11:25:41Z

https://ci.opm-project.org/job/opm-simulators-PR-builder/9194/

jenkins also shows that we need the serializeOp function. I do not have HDF5 support enabled, so it was not spotted.

GitPaean added the manual:irrelevant This PR is a minor fix and should not appear in the manual label Dec 3, 2025

GitPaean force-pushed the struct_group_target branch from 1a23d6f to 80e4f79 Compare December 4, 2025 09:00

GitPaean force-pushed the struct_group_target branch 2 times, most recently from 80d910d to f540381 Compare December 4, 2025 11:56

GitPaean force-pushed the struct_group_target branch from f540381 to a5b6685 Compare December 4, 2025 12:16

GitPaean force-pushed the struct_group_target branch 2 times, most recently from a5b6685 to 08d7c3b Compare December 5, 2025 08:43

akva2 mentioned this pull request Dec 5, 2025

failure_report: add parallel execution #6665

Merged

GitPaean force-pushed the struct_group_target branch from 08d7c3b to cff6970 Compare December 7, 2025 21:09

GitPaean force-pushed the struct_group_target branch 2 times, most recently from 2740766 to a532a02 Compare December 9, 2025 12:10

GitPaean marked this pull request as ready for review December 9, 2025 12:12

GitPaean requested review from bska and steink December 9, 2025 12:12

steink approved these changes Dec 11, 2025

View reviewed changes

bska reviewed Dec 11, 2025

View reviewed changes

GitPaean force-pushed the struct_group_target branch from a532a02 to 7852484 Compare December 12, 2025 10:10

GitPaean marked this pull request as draft December 15, 2025 08:34

GitPaean force-pushed the struct_group_target branch from 7852484 to 7e6527e Compare December 15, 2025 08:35

introducing GroupTarget struct for SingleWellState

7e6527e

so it can record the group name that results the group target, the group control mode (either injecting or producing) of the group, and also the target value enforced to this well.

introducing GroupTarget struct for SingleWellState #6654

Are you sure you want to change the base?

introducing GroupTarget struct for SingleWellState #6654

Conversation

GitPaean commented Dec 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

GitPaean commented Dec 3, 2025

Uh oh!

GitPaean commented Dec 3, 2025

Uh oh!

GitPaean commented Dec 3, 2025

Uh oh!

GitPaean commented Dec 3, 2025

Uh oh!

steink commented Dec 3, 2025

Uh oh!

GitPaean commented Dec 4, 2025

Uh oh!

GitPaean commented Dec 4, 2025

Uh oh!

GitPaean commented Dec 4, 2025

Uh oh!

GitPaean commented Dec 4, 2025

Uh oh!

GitPaean commented Dec 4, 2025

Uh oh!

GitPaean commented Dec 4, 2025

Uh oh!

GitPaean commented Dec 4, 2025

Uh oh!

GitPaean commented Dec 4, 2025

Uh oh!

GitPaean commented Dec 4, 2025

Uh oh!

GitPaean commented Dec 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

GitPaean commented Dec 5, 2025

Uh oh!

GitPaean commented Dec 7, 2025

Uh oh!

GitPaean commented Dec 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

GitPaean commented Dec 9, 2025

Uh oh!

steink left a comment

Choose a reason for hiding this comment

Uh oh!

bska left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

GitPaean commented Dec 15, 2025

Uh oh!

GitPaean commented Dec 15, 2025

Uh oh!

GitPaean commented Dec 15, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

GitPaean commented Dec 3, 2025 •

edited

Loading

GitPaean commented Dec 4, 2025 •

edited

Loading

GitPaean commented Dec 7, 2025 •

edited

Loading