OPM / opm-core

Collection of utilities, solvers and other components.
http://www.opm-project.org
GNU General Public License v3.0
44 stars 50 forks source link

Add energy phase #1192

Closed andlaus closed 6 years ago

andlaus commented 6 years ago

this adds a energy "phase" analogous to solvents and polymer to PhaseUsage. It also fixes a compiler warning which appears because the Phase class of opm-parser now has an 'ENERGY' enum.

andlaus commented 6 years ago

jenkins build this with downstreams please

atgeirr commented 6 years ago

I'm not sure that we want to use the term energy here. While it is the conserved quantity, "heat" would also be appropriate I think. @hnil what term would you use (from a physics point of view)?

I am also not sure why we need to modify this class, is it only to communicate the "what model the deck asked for" information?

GitPaean commented 6 years ago

We are having problem with what is phases and what is components (for example, polymer, surfacant), energy is neither phase or component. Maybe it is time trying to clarify the concepts here and also make the future extension/development easier? As a blackoil based simulator, we will have mass conservation related to phases for sure. And we can have some components transport along with the phases, like polymer. Energy is a separate concept and playing a different role, can be one by itself.

BTW: I think heat is used when discussing heat transfer, I do not think it is conserved.

andlaus commented 6 years ago

While it is the conserved quantity, "heat" would also be appropriate I think.

@atgeirr: IMO it wouldn't because what's "heat"? is it internal energy? it surely isn't a conservation quantity in physics. Also be aware that friction is implicitly accounted for by the fact that the storage term considers internal energy while fluxes are about enthalpies. (I guess that you'll agree that the kinetic energy of the fluids is pretty much negligible, which is also acknowledged by the fact that there is no acceleration in the mass conservation equations.)

energy is neither phase or component

@GitPaean: if you consider "component" synonymous with "conservation quantity" it is a component. I agree that the nomenclature is quite a bit abused, but this is not a new problem introduced by energy. In other words thats material for a future cleanup project.

And we can have some components transport along with the phases, like polymer.

... or, in the not too distant future, energy.

BTW: I think heat is used when discussing heat transfer, I do not think it is conserved.

yes, but as I see this, "heat transfer" is just a shortcut for "energy flux due to the thermal gradient" (or "heat gradient" if you prefer this term)

andlaus commented 6 years ago

can this be merged? #1195 caused a minor conflict, but that is fixed now.

atgeirr commented 6 years ago

You have me convinced that "energy" is a good name to use. However, I think that the hack involving MaxNumPhases etc. is rather horrible, and will lead to problems. What about handling energy the same way as with polymer/solvent, i.e. only the has_energy flag? We do conserve those quantities as well, and mapping of indices is done further down the line anyway.

andlaus commented 6 years ago

basically the only part of the code which still needs PhaseUsage this is flow's well model. If you have a look, you'll notice that polymer and solvent do currently not play nicely together. if this is acceptable for energy as well (I'd argue that at least the polymer extension should work with energy enabled), it is not a big deal to remove the MaxNumPhases hack, in the other case the wells code for energy will become quite a bit uglier than it needs to be.

atgeirr commented 6 years ago

I see your point. So the correct solution is to update MaxNumPhases, a quick search revealed that it is not used in very many places (3 files in opm-simulators, a few more in opm-core). Perhaps even change it to incorporate polymer and solvent as "phases"?

andlaus commented 6 years ago

Perhaps even change it to incorporate polymer and solvent as "phases"?

For extending the proposed hack for energy to polymer and solvent, I agree. For changing MaxNumPhases, I'm not sure if this is worth the effort; in particular because this would require quite a few changes in flow_legacy, right?

atgeirr commented 6 years ago

this would require quite a few changes in flow_legacy, right?

Not that many, I believe, but having thought about this it's ok to do it in two steps.

The first step would be to add polymer and solvent treatment to this PR similar to how you did energy, as you suggest. However I think you should define a new constant, MaxNumConservedQuantities or similar, that is the real size of things instead of using MaxNumPhases + literal number.

The second step would be to drop MaxNumPhases, but that would be a separate thing.

andlaus commented 6 years ago

okay, polymer and solvent are now handled just like energy

andlaus commented 6 years ago

jenkins build this with downstreams please

GitPaean commented 6 years ago

but this is not a new problem introduced by energy. In other words thats material for a future cleanup project.

Sorry for not responding this earlier. It is not a new problem introduced by this PR. It needs probably a future cleaning up project. It is one of the major obstacles prevent OPM to be a versatile simulator in my opinion.

andlaus commented 6 years ago

jenkins build this with downstreams please

andlaus commented 6 years ago

I do not get that error jenkins gets on my machine because the oil-gas test case seems not to be available in master yet. probably there were some races on last time. let's try again...

andlaus commented 6 years ago

jenkins build this with downstreams please

andlaus commented 6 years ago

I found the issue (usage of incorrect variable as index), I just wonder why this only happened with a single MPI test. I suppose that means that the number of stupid bugs possible is truly infinite.

andlaus commented 6 years ago

jenkins build this with downstreams please

andlaus commented 6 years ago

arg pushed wrong version

andlaus commented 6 years ago

jenkins build this with downstreams please

atgeirr commented 6 years ago

Happy with this now, merging!