Adding trapped mass as output from compositional statistics #2052
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ies' into jafranc/feat/add-mobile-comp-stat
francoishamon
approved these changes
Sep 9, 2022
There was a problem hiding this comment.
Thanks very much @jafranc this is very useful! I just made the cosmetic comments about naming that we discussed today. Let me know when you need a rebaseline.
src/coreComponents/constitutive/relativePermeability/BrooksCoreyBakerRelativePermeability.cpp
Outdated
Show resolved
Hide resolved
src/coreComponents/constitutive/relativePermeability/BrooksCoreyBakerRelativePermeability.hpp
Outdated
Show resolved
Hide resolved
src/coreComponents/constitutive/relativePermeability/BrooksCoreyBakerRelativePermeability.hpp
Outdated
Show resolved
Hide resolved
src/coreComponents/constitutive/relativePermeability/RelativePermeabilityBase.cpp
Outdated
Show resolved
Hide resolved
src/coreComponents/constitutive/relativePermeability/RelativePermeabilityBase.cpp
Outdated
Show resolved
Hide resolved
src/coreComponents/physicsSolvers/fluidFlow/CompositionalMultiphaseStatistics.cpp
Outdated
Show resolved
Hide resolved
src/coreComponents/physicsSolvers/fluidFlow/IsothermalCompositionalMultiphaseBaseKernels.hpp
Outdated
Show resolved
Hide resolved
src/coreComponents/physicsSolvers/fluidFlow/IsothermalCompositionalMultiphaseBaseKernels.hpp
Outdated
Show resolved
Hide resolved
Comment on lines
630
to
645
| void TableRelativePermeabilityHysteresis::updateTrappedPhaseVolFraction( arrayView2d< real64 const, compflow::USD_PHASE > const & phaseVolFraction ) const | ||
| { | ||
|
|
||
| arrayView2d< real64, compflow::USD_PHASE > phaseTrapped = m_phaseTrapped.toView(); | ||
| arrayView2d< real64, compflow::USD_PHASE > phaseCrit = m_phaseCriticalVolFraction.toView(); | ||
|
|
||
| localIndex const numElems = phaseVolFraction.size( 0 ); | ||
| integer const numPhases = numFluidPhases(); | ||
| forAll< parallelDevicePolicy<> >( numElems, [=] GEOSX_HOST_DEVICE ( localIndex const ei ) { | ||
| for( integer ip = 0; ip < numPhases; ++ip ) | ||
| { | ||
| phaseTrapped[ei][ip] = LvArray::math::min( phaseVolFraction[ei][ip], | ||
| phaseCrit[ei][ip] ); | ||
| } | ||
| } ); | ||
| } |
There was a problem hiding this comment.
I have a suggestion to save an array2d (but I may be wrong).
Currently:
- You compute
phaseCriticalVolFractionin the hysteretic kernels - At the end of the time step, you call
updateTrappedPhaseVolFractionto take the min ofphaseVolFractionandphaseCriticalVolFraction
Instead, would it be possible to do the following:
- You remove
phaseCriticalVolFractionfrom the classes - You pass
phaseTrappedVolFractionto the hysteretic kernels - Inside them, you compute and use
Scrtas before - At the end of the hysteretic kernels, you do:
phaseTrappedVolFraction = min( S, Scrt ) - You remove
updateTrappedPhaseVolFraction
but I may be missing something.
There was a problem hiding this comment.
Great idea ! should be done in 8052c80.
It might not be ideal as it implies quite some code duplication over the compute() functions of all relperm classes.
I tried though to have compute() pure virtual in relativePermeabilityBase instead of update and use update to trigger compute() and updateTrappedVolFrac() but bump into the change of interface of compute() for TableRelativePermeabilityHysteresis. That would imply using some adaptation I guess.
We might save that for future refactoring if the idea was worth pursuing.
herve-gross
approved these changes
Sep 9, 2022
…t' into jafranc/feat/add-mobile-comp-stat
…t' into jafranc/feat/add-mobile-comp-stat
# Conflicts: # src/coreComponents/physicsSolvers/fluidFlow/CompositionalMultiphaseStatistics.cpp # src/coreComponents/physicsSolvers/fluidFlow/IsothermalCompositionalMultiphaseBaseKernels.hpp
francoishamon
added
flag: requires rebaseline
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR aims at adding trapped mass in the statistics output from
CompositionalMultiphaseStatistics.It involves
m_phaseMinVolFractiontoRelativePermeabilityBasewith accessorm_phaseTrappedanarray2d<real64, compflow::LAYOUT_PHASE>to store per-element minimal critical saturationThis last step is needed as for hysteretic relperm minimal critical saturation is not global to the domain.
Rebaseline done in https://github.com/GEOSX/integratedTests/pull/260