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

Partitioning tracers #5268

Merged
merged 8 commits into from
Jun 14, 2024
Merged

Partitioning tracers #5268

merged 8 commits into from
Jun 14, 2024

Conversation

svenn-t
Copy link
Contributor

@svenn-t svenn-t commented Apr 3, 2024

Extended the tracer model to include partitioning of tracers into free and solution states whenever dissolved gas/vaporized oil exist. Essentially the tracer linear system has been extended with equations for free and solution tracers with simple mass transfer coupling between them. Both free and solution tracers are now outputted in restart files.

The PR is in draft mode to discuss and further improve/develop the tracer model. Per now it seems the results for the solution tracer part generally does not match reference results (e.g. in the gas tracer test); only matches for simple test cases (e.g., one cell partitioning tracers). However, results for the free tracer part does generally match reference results and are also improved over current master implementation.

Note: Due to the current implementation of the TRACERS keyword, only option 2 (=number of water tracers) is used in the simulations. So this option must be set regardless of which type of tracer that is used. Correcting this seemed a bit over my head at the moment, but when/if this draft becomes a real PR, the keyword behavior must be fixed.

Depends on: OPM/opm-common#4001

@svenn-t
Copy link
Contributor Author

svenn-t commented May 16, 2024

With latest PR update, the tracer concentration, free and solution, match reference results quite well. Should be tested a bit more before making it a proper PR.

Note: Due to the current implementation of the TRACERS keyword, only option 2 (=number of water tracers) is used in the simulations. So this option must be set regardless of which type of tracer that is used. Correcting this seemed a bit over my head at the moment, but when/if this draft becomes a real PR, the keyword behavior must be fixed.

Note: Updates in OPM/opm-common#4001 should fix this. In addition, summary keywords for free and solution tracers (e.g., WTPRF or WTPRS) are now supported.

@svenn-t
Copy link
Contributor Author

svenn-t commented May 21, 2024

Note: Updates in OPM/opm-common#4001 should fix this. In addition, summary keywords for free and solution tracers (e.g., WTPRF or WTPRS) are now supported.

Changes related to the TRACERS keyword have been split off to a new PR: OPM/opm-common#4072

Solve an extended linear system with free and solution tracers with mass transfer coupling term.
Additionally, store separate well terms for free and solution tracers
-Only output or restart solution tracers for gas/oil tracers with DISGAS/VAPOIL enabled (no solution tracers in water phase!).
-Initial tracers (free/solution) will be set to zero initially if TBLK/TVDP is not given.
- Do not calculate mass transfer between free and solution tracers if it is not necessary.
-Calculate well rates using updated tracer concentrations
@svenn-t svenn-t marked this pull request as ready for review June 10, 2024 12:53
@bska
Copy link
Member

bska commented Jun 11, 2024

jenkins build this please

Copy link
Member

@totto82 totto82 left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution. I mostly have minor comment. The performance should be checked on norne since this increase the system size to 2

// ///
// OBS: below code will give runtime error since we cannot access intensive quantities at timeIdx=1
// //
Scalar fVolume1 = computeFreeVolume_(tr.phaseIdx_, I1, 1);
Copy link
Member

Choose a reason for hiding this comment

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

Does this mean that we dont support enableStorageCache = false

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Previously I got errors when setting enableStorageCache = false, but this does not seem to be the case anymore. I have removed the comment from the code.

}

Scalar rate_f = rate - rate_s;
if (rate_f > 0) {
Copy link
Member

Choose a reason for hiding this comment

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

Can we get the free rate from ws.perf_data directly?

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 looked but couldn't find any at least.

this->wellFreeTracerRate_.at(std::make_pair(eclWell.name(),this->wellfname(tr.idx_[tIdx]))) += rate_f*wtracer[tIdx];
if (eclWell.isMultiSegment()) {
this->mSwTracerRate_[std::make_tuple(eclWell.name(), this->name(tr.idx_[tIdx]), eclWell.getConnections().get(i).segment())] =
rate_f*wtracer[tIdx];
Copy link
Member

Choose a reason for hiding this comment

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

Should this be +=

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 changed to += here and elsewhere (initialized mSwTracerRate_ to zero also)


if ((tr.concentration_[tIdx][globalDofIdx][0] - dx[tIdx][globalDofIdx][0] < 0.0)
|| Sf < 1e-6)
tr.concentration_[tIdx][globalDofIdx][0] = 0.0;
Copy link
Member

Choose a reason for hiding this comment

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

I think it would be better to use Scalar tol_gas_sat = 1e-6 to make it clear what it is.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed.

rate_f * tr.concentration_[tIdx][I][0];
if (eclWell.isMultiSegment()) {
this->mSwTracerRate_[std::make_tuple(eclWell.name(), this->name(tr.idx_[tIdx]), eclWell.getConnections().get(i).segment())] =
rate_f * tr.concentration_[tIdx][I][0];
Copy link
Member

Choose a reason for hiding this comment

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

+=?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed

rate_s * tr.concentration_[tIdx][I][1];
if (eclWell.isMultiSegment()) {
this->mSwTracerRate_[std::make_tuple(eclWell.name(), this->name(tr.idx_[tIdx]), eclWell.getConnections().get(i).segment())] =
rate_s * tr.concentration_[tIdx][I][1];
Copy link
Member

Choose a reason for hiding this comment

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

+=?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed

}

Scalar rate_f = rate - rate_s;
if (rate_f < 0) {
Copy link
Member

Choose a reason for hiding this comment

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

same comment as for injector

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See comment above

@totto82
Copy link
Member

totto82 commented Jun 12, 2024

benchmark please

@ytelses
Copy link

ytelses commented Jun 12, 2024

Benchmark result overview:

Test Configuration Relative
opm-git OPM Benchmark: drogon - Threads: 1 0.99
opm-git OPM Benchmark: drogon - Threads: 8 1.004
opm-git OPM Benchmark: punqs3 - Threads: 1 1.028
opm-git OPM Benchmark: punqs3 - Threads: 8 0.991
opm-git OPM Benchmark: smeaheia - Threads: 1 0.996
opm-git OPM Benchmark: smeaheia - Threads: 8 1.006
opm-git OPM Benchmark: spe10_model_1 - Threads: 1 1.018
opm-git OPM Benchmark: spe10_model_1 - Threads: 8 0.999
opm-git OPM Benchmark: flow_mpi_extra - Threads: 1 0.997
opm-git OPM Benchmark: flow_mpi_extra - Threads: 8 1.004
opm-git OPM Benchmark: flow_mpi_norne - Threads: 1 1
opm-git OPM Benchmark: flow_mpi_norne - Threads: 8 1.002
opm-git OPM Benchmark: flow_mpi_norne_4c_msw - Threads: 1 0.989
opm-git OPM Benchmark: flow_mpi_norne_4c_msw - Threads: 8 0.991
  • Speed-up = Total time master / Total time pull request. Above 1.0 is an improvement. *

View result details @ https://www.ytelses.com/opm/?page=result&id=2508

@totto82
Copy link
Member

totto82 commented Jun 13, 2024

jenkins build this please

@totto82
Copy link
Member

totto82 commented Jun 14, 2024

Thanks for the contribution. Looks good to me

@totto82 totto82 merged commit 151bba4 into OPM:master Jun 14, 2024
1 check passed
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.

4 participants