-
Notifications
You must be signed in to change notification settings - Fork 89
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
feat: Simplify region statistics output in HDF5 #3314
base: develop
Are you sure you want to change the base?
Conversation
…pace and pack in that space
…pe(referenceAsView)
src/coreComponents/unitTests/fluidFlowTests/testFlowStatistics.cpp
Outdated
Show resolved
Hide resolved
src/coreComponents/physicsSolvers/fluidFlow/CompositionalMultiphaseStatistics.hpp
Outdated
Show resolved
Hide resolved
src/coreComponents/physicsSolvers/fluidFlow/CompositionalMultiphaseStatistics.cpp
Outdated
Show resolved
Hide resolved
src/coreComponents/physicsSolvers/fluidFlow/StatOutputController.cpp
Outdated
Show resolved
Hide resolved
src/coreComponents/physicsSolvers/fluidFlow/unitTests/testStatOutputController.cpp
Outdated
Show resolved
Hide resolved
for( string const & regionName : regionNames ) | ||
{ | ||
ElementRegionBase & region = elemManager.getRegion( regionName ); | ||
string const regionStatPath = GEOS_FMT( "{}/regionStatistics", region.getPath() ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you should use a viewKey
here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor last comments
@@ -270,7 +364,7 @@ void CompositionalMultiphaseStatistics::computeRegionStatistics( real64 const ti | |||
array1d< real64 > subRegionTrappedPhaseMass( numPhases ); | |||
array1d< real64 > subRegionImmobilePhaseMass( numPhases ); | |||
array1d< real64 > subRegionRelpermPhaseMass( numPhases ); | |||
array2d< real64 > subRegionComponentMass( numPhases, numComps ); | |||
array2d< real64 > subRegiondissolvedComponentMass( numPhases, numComps ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
camelCase
{ | ||
ElementRegionBase & region = elemManager.getRegion( regionName ); | ||
string const regionStatPath = GEOS_FMT( "{}/{}", region.getPath(), | ||
SinglePhaseStatistics::viewKeyStruct::regionStatisticsString() ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can move this viewKey
in FieldStatisticsBase
if you add a comment on it to say it only exists for statistics that are computed for regions.
GEOS_LOG_LEVEL_INFO_RANK_0( logInfo::Statistics, | ||
GEOS_FMT( "{} Mobile phase mass (metric 2): {} {}", | ||
statPrefix, mobilePhaseMass, massUnit ) ); | ||
statPrefix, mobilePhaseMass, massUnit )); | ||
|
||
GEOS_LOG_LEVEL_INFO_RANK_0( logInfo::Statistics, | ||
GEOS_FMT( "{} Component mass: {} {}", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should let appear the "dissolved" term in the logs.
(cherry picked from commit eb27814fd004bc6cd0fcfe905e4b6c41b8ee8ea5)
…S-DEV/GEOS into feature/dudes/output-regions
…S-DEV/GEOS into feature/dudes/output-regions
this PR is based on the work of @wrtobin & @untereiner : Memory Space Aware Packing , Export info embedded on regions
With the current impl, in order to output region statistics we would have to create :
<CompositionalMultiphaseStatistics>
component + 1<PeriodicEvent>
<TimeHistory>
perregion
with each desired statistics fields (hidden to the user) + 1<periodicEvent>
per<TimeHistory>
<PackCollection>
perfield
and 1<PeriodicEvent>
per<packCollection>
With multiple regions this could easily lead to
hundred
of components to create and maintain manuallyWith the controller component proposed in this PR, we would have all
<PackCollection>
,<TimeHistory>
and their<PeriodicEvent>
automatically created / managed :