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

WIP Add flow_dyn #2874

Closed
wants to merge 3 commits into from
Closed

WIP Add flow_dyn #2874

wants to merge 3 commits into from

Conversation

totto82
Copy link
Member

@totto82 totto82 commented Oct 26, 2020

Adds a dynamic flow version that only takes number of equations as parameter

The goal is to reduce the number of flow versions i.e reduce compile time

The drawback is a slight overhead in the run time. Initial testing indicates

  1. Extra cost of using dynamic mappings and if evaluations < 1% for norne
  2. Extra cost of adding enableSolvent unconditionally for norne. ~5%. The solvent code is never called and the number of equation is still 3, but the intensive quantities size is increased to carry solvent related stuff.
  3. Extra cost of using dynamic flow for oil-water olympus. ~2%.. The dynamic flow currently always allocating three phases in the fluid state.

This should neither alter the results nor the runtime for flow.

Some optimization could potentially further reduce the overhead associated with the dynamic blackoil indices code.

Depends on OPM/opm-models#631

Tor Harald Sandve added 3 commits October 26, 2020 15:01
Adds a dynamic flow version that only takes number of equations as paramter
The goal is to reduce the number of flow versions i.e reduce compile time
The drawback is a slight overhead in the runtime. Initial testing indicates
in the range of 2-3%. This may change....
@joakim-hove
Copy link
Member

I kind of like the current compile time?!

@totto82
Copy link
Member Author

totto82 commented Oct 26, 2020

I kind of like the current compile time?!

It may not be worth the extra complication and overhead in the code, but the ever increasing number of flow version worried me enough to test an alternative. I know some colleagues have some more versions in making and also at TNO they are planning more.

The suggested approach allows for both compile time and dynamic versions so we can get the best from both words.

@joakim-hove
Copy link
Member

The suggested approach allows for both compile time and dynamic versions so we can get the best from both words.

Jokes in writing is always difficult; I am positive to any suggestion which can reduce the current compile time!

@alfbr
Copy link
Member

alfbr commented Oct 26, 2020

Cool, I need to let it sink in a bit before fully realizing the implications, but definitely positive from the outset :-)

@bska
Copy link
Member

bska commented Mar 21, 2021

@totto82 : If this work is still relevant, how much effort would it be to rebase the patch onto the current master and make the changes necessary—if any—to make it run

@totto82
Copy link
Member Author

totto82 commented Mar 22, 2021

I will close the PR. It was for mostly discussion and testing. I think the bullet 1) above should be carried out. i.e. to avoid compile time determination if it is a gas-water, oil-water or a oil-gas case. The rest should probably not change.

@totto82 totto82 closed this Mar 22, 2021
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