Skip to content

Adding the ROE summary vector#4833

Merged
bska merged 1 commit into
OPM:masterfrom
vkip:roe_smry
Nov 13, 2025
Merged

Adding the ROE summary vector#4833
bska merged 1 commit into
OPM:masterfrom
vkip:roe_smry

Conversation

@vkip
Copy link
Copy Markdown
Member

@vkip vkip commented Nov 13, 2025

No description provided.

@vkip vkip added the manual:enhancement This is an enhancement/improvent that needs to be documented in the manual label Nov 13, 2025
@vkip
Copy link
Copy Markdown
Member Author

vkip commented Nov 13, 2025

jenkins build this please

@vkip
Copy link
Copy Markdown
Member Author

vkip commented Nov 13, 2025

All test failures seem only due to the addition of the ROE summary vectors (ROE:1 and ROE:2)

@bska
Copy link
Copy Markdown
Member

bska commented Nov 13, 2025

All test failures seem only due to the addition of the ROE summary vectors (ROE:1 and ROE:2)

Agreed. I'll just double check some of the failures which had multiple hits for these.

@bska
Copy link
Copy Markdown
Member

bska commented Nov 13, 2025

jenkins build this ignore_extra please

Copy link
Copy Markdown
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.

All test failures seem only due to the addition of the ROE summary vectors (ROE:1 and ROE:2)

Agreed. I'll just double check some of the failures which had multiple hits for these.

Indeed, those are the only test failures so once we've addressed the defining expression of the efficiency factor I'll update the reference solutions.

Comment thread opm/output/eclipse/Summary.cpp Outdated

const auto initial = initial_inplace.get( region_name, Opm::Inplace::Phase::OIL, args.num );
const auto current = args.inplace.get( region_name, Opm::Inplace::Phase::OIL, args.num );
return { (initial - current) / current, measure::identity };
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.

This definition looks at little suspicious to me. Did you mean to divide by initial instead? In the limit as current tends to zero (full recovery), we'll get an efficiency factor approaching infinity and that's unlikely to be useful.

Moreover, since this is a per-region quantity, we should also guard against initial == 0 which could be the case in the gas cap or in the water zone.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

🤦‍♂️ Should be initial of course, and will check for zero. Thanks!

@vkip
Copy link
Copy Markdown
Member Author

vkip commented Nov 13, 2025

jenkins build this please

Comment thread opm/output/eclipse/Summary.cpp Outdated

const auto initial = initial_inplace.get( region_name, Opm::Inplace::Phase::OIL, args.num );
if (initial < 1.0e-15)
return 0.0;
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.

This statement gives a build failure. I think you meant

return zero;

instead.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yup, glad one of us is awake. Thanks again!

@vkip
Copy link
Copy Markdown
Member Author

vkip commented Nov 13, 2025

jenkins build this please

@bska
Copy link
Copy Markdown
Member

bska commented Nov 13, 2025

jenkins build this update_data please

jenkins4opm pushed a commit to jenkins4opm/opm-tests that referenced this pull request Nov 13, 2025
Reason: PR OPM/opm-common#4833

opm-common     = ca9701261c5beb55684eace70f4bc9d025d48758
opm-grid       = a0c0fa539c264e9308e65795a9614bb11f2d5309
opm-simulators = ca9701261c5beb55684eace70f4bc9d025d48758

### Changed Tests ###

  * gconprod_t1l
  * gconprod_t1w
  * gconprod_t2g
  * gconprod_t2o
  * base_wt_tracer
  * base_wt_tracer(restart)
  * 01_wgrupcon
  * 02_wgrupcon
  * 6_uda_model5_stdw
  * 6_uda_model5_stdw(restart)
@bska
Copy link
Copy Markdown
Member

bska commented Nov 13, 2025

jenkins build this opm-tests=1429 please

bska added a commit to OPM/opm-tests that referenced this pull request Nov 13, 2025
Copy link
Copy Markdown
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.

Thanks a lot for the updates. This looks good to me now and as the new reference solutions have been installed on the CI system, I'll merge into master.

@bska bska merged commit 36ed527 into OPM:master Nov 13, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

manual:enhancement This is an enhancement/improvent that needs to be documented in the manual

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants