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

testing removing fucntion invertMatrix #831

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

GitPaean
Copy link
Member

No description provided.

@GitPaean
Copy link
Member Author

jenkins build this opm-simulators=4896 please

@blattms
Copy link
Member

blattms commented Sep 26, 2023

If I recall correctly, we have this here to get better performance. In Dune there was a specialization missing and since DenseMatrix was introduced the optimized invert method was not picked up. I can be missing something.

We definitely will need to benchmark the changes here to be sure that the prerequisites do not hold anymore.

@GitPaean
Copy link
Member Author

benchmark please

@GitPaean
Copy link
Member Author

Did not go through the jenkins failures in detail. But it looks like only small fluctuations. Very likely that we can go ahead to remove the MatrixBlock unless more information shows the opposite.

I do not know where to find the benchmark results anymore.

@blattms
Copy link
Member

blattms commented Sep 28, 2023

I have notified Michael about the benchmark problems.

In addition we need to do some manual benchmarking with more than 3 phases, too.

@GitPaean
Copy link
Member Author

In addition we need to do some manual benchmarking with more than 3 phases, too.

For the blackoil cases, StandardWell has 4 primary variables, so the 4X4 inversion might be used. But I do not think it will affect the performance in a noticeable way.

For other cases, maybe the cases with 4 primary variables, like blackoil + polymer might use it, we can check it. Other than that, there is not much 4X4 matrices involved I believe.

@blattms
Copy link
Member

blattms commented Sep 28, 2023

benchmark please

@GitPaean
Copy link
Member Author

jenkins build this opm-simulators=4896 please

@GitPaean
Copy link
Member Author

benchmark opm-simulators=4896 please

@ytelses
Copy link

ytelses commented Jun 17, 2024

Benchmark result overview:

Test Configuration Relative
opm-git OPM Benchmark: drogon - Threads: 1 1
opm-git OPM Benchmark: drogon - Threads: 8 0.978
opm-git OPM Benchmark: punqs3 - Threads: 1 1.013
opm-git OPM Benchmark: punqs3 - Threads: 8 1.003
opm-git OPM Benchmark: smeaheia - Threads: 1 1.018
opm-git OPM Benchmark: smeaheia - Threads: 8 1.007
opm-git OPM Benchmark: spe10_model_1 - Threads: 1 1.001
opm-git OPM Benchmark: spe10_model_1 - Threads: 8 1
opm-git OPM Benchmark: flow_mpi_extra - Threads: 1 1.009
opm-git OPM Benchmark: flow_mpi_extra - Threads: 8 1.006
opm-git OPM Benchmark: flow_mpi_norne - Threads: 1 0.985
opm-git OPM Benchmark: flow_mpi_norne - Threads: 8 0.986
opm-git OPM Benchmark: flow_mpi_norne_4c_msw - Threads: 1 1.008
opm-git OPM Benchmark: flow_mpi_norne_4c_msw - Threads: 8 0.995
  • 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=2515

@GitPaean
Copy link
Member Author

benchmark opm-simulators=4896 please

@OPM OPM deleted a comment from ytelses Jun 17, 2024
@OPM OPM deleted a comment from ytelses Jun 17, 2024
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.

3 participants