Skip to content

Make available current Production/Injection Group Control parameters for use in Restart#2459

Merged
bska merged 7 commits intoOPM:masterfrom
jalvestad:group_constraints_summary
Mar 19, 2020
Merged

Make available current Production/Injection Group Control parameters for use in Restart#2459
bska merged 7 commits intoOPM:masterfrom
jalvestad:group_constraints_summary

Conversation

@jalvestad
Copy link
Contributor

@jalvestad jalvestad commented Mar 12, 2020

This PR has changes that make parameters: currentProductionGroupControl and currentInjectionGroupControl available for writing Eclipse compatible restart files and for use as Summary file data. The content of the restart files will be changed as a consequence of using this information, to be included in a separate PR in opm-common.

I would like to get this pull request merged soon since it is required to use together with other PRs for further work / improvment of the eclipse compatible restart files.

@jalvestad jalvestad force-pushed the group_constraints_summary branch from 1fbfb92 to 6a65933 Compare March 16, 2020 09:40
@bska
Copy link
Member

bska commented Mar 16, 2020

jenkins build this please

@bska
Copy link
Member

bska commented Mar 16, 2020

jenkins build this please (failed)

Right, the PR also depends on OPM/opm-common#1408 and cannot be tested in isolation.

@bska
Copy link
Member

bska commented Mar 16, 2020

jenkins build this opm-common=1592 please

Copy link
Member

@bska bska left a comment

Choose a reason for hiding this comment

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

This looks good to me. Just a couple of comments concerning unused parameters and local variables.

return wellDat;
}

Opm::data::Group groupData(const int reportStepIdx, Opm::Schedule& sched) const {
Copy link
Member

Choose a reason for hiding this comment

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

The parameters here are unused. I suggest writing the declaration as

Opm::data::Group groupData(const int /* reportStepIdx */, const Opm::Schedule& /* sched */) const {

In other words, without naming the parameters. Doing so will avoid warnings about unused parameters.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for the comment, I agree and this change has now been done and pr has been updated.

Copy link
Member

Choose a reason for hiding this comment

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

Thank you for the comment, I agree and this change has now been done and pr has been updated.

Thank you very much. Did you want to address my other comment too, or do you consider this PR done now?

bool enableDoublePrecisionOutput = EWOMS_GET_PARAM(TypeTag, bool, EclOutputDoublePrecision);
const Opm::data::Solution& cellData = collectToIORank_.isParallel() ? collectToIORank_.globalCellData() : localCellData;
const Opm::data::Wells& wellData = collectToIORank_.isParallel() ? collectToIORank_.globalWellData() : localWellData;
const Opm::data::Group& groupData = collectToIORank_.isParallel() ? collectToIORank_.globalGroupData() : localGroupData;
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this groupData object is actually used, since the relevant quantities have already been calculated in evalSummaryState() and put into the summaryState() object. The restart writer can retrieve the values from there.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree and I have removed this object from the code - thank you for the correction!

Copy link
Member

Choose a reason for hiding this comment

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

I agree and I have removed this object from the code

Thanks a lot! I'll launch a data-update now. I expect we'll be able to merge this later today.

@bska
Copy link
Member

bska commented Mar 19, 2020

jenkins build this opm-common=1592 update_data please

jenkins4opm pushed a commit to jenkins4opm/opm-tests that referenced this pull request Mar 19, 2020
Reason: OPM/opm-common#1592
        OPM/opm-simulators#2459

opm-common     = 1aa28ed927a5a912d2819dbe24550ff8b7a458d1
opm-grid       = 1d0697f99f3738b3c01707aa8b1dd85da448cc8b
opm-material   = 16fcd186c70f170c80431c97e802eabcc548feae
opm-models     = 214100686144f801c4ff56c016bf71195c789c25
opm-simulators = 29bd764db12a707a4917b879ca041ea9f0cd91a6
@bska
Copy link
Member

bska commented Mar 19, 2020

Green from upstream test run. I'll merge this into master now. Thanks a lot.

@bska bska merged commit fda4a29 into OPM:master Mar 19, 2020
@jalvestad jalvestad deleted the group_constraints_summary branch April 7, 2020 07:53
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.

2 participants