-
Notifications
You must be signed in to change notification settings - Fork 90
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: Transmissibility output #3091
Conversation
outputFile << time << "," | ||
<< conn.globalId[0] << "," | ||
<< conn.globalId[1] << "," | ||
<< conn.transmissibility[0] << "," |
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.
not sure it makes much sense to print the same transmissibility value twice but it's ok, not a big deal
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.
Maybe it will be more useful to output two half-transmissibilities.
FiniteVolumeManager const & fvManager = numericalMethodManager.getFiniteVolumeManager(); | ||
FluxApproximationBase const & fluxApprox = fvManager.getFluxApproximation( m_solver->getDiscretizationName() ); | ||
|
||
fluxApprox.forStencils< CellElementStencilTPFA >( mesh, [&]( auto const & stencil ) |
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.
could you please check if all that will work for https://github.com/GEOS-DEV/GEOS/blob/develop/inputFiles/compositionalMultiphaseFlow/co2_hybrid_1d.xml ?
not sure hybrid discretization has the same things as TPFA
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 suppose the key here is that fvManager.getFluxApproximation( m_solver->getDiscretizationName() );
might return null if the solver doesn't have a flux approximation. So I would add a validator (postProcessInput()
) to the solverName
field of this to ensure that the solver provided has the appropriate flux approximation.
FiniteVolumeManager const & fvManager = numericalMethodManager.getFiniteVolumeManager(); | ||
FluxApproximationBase const & fluxApprox = fvManager.getFluxApproximation( m_solver->getDiscretizationName() ); | ||
|
||
fluxApprox.forStencils< CellElementStencilTPFA >( mesh, [&]( auto const & stencil ) |
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 suppose the key here is that fvManager.getFluxApproximation( m_solver->getDiscretizationName() );
might return null if the solver doesn't have a flux approximation. So I would add a validator (postProcessInput()
) to the solverName
field of this to ensure that the solver provided has the appropriate flux approximation.
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.
It seems that we are trying to only write a file for rank 0 but I don't see when you gather all stencil data there? This would only work for small cases....
In any case, why not using the timehistory and writing in a parallel format instead? @wrtobin opinions?
Besides debugging I don't really see why someone would want to write out transmissibilities.
Point is to allow to do an HDF output, and remove the CSV one.
…end. Point is to solve a bug where the StencilDataCollection buffer size are not known for the HDF chunk sizes.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #3091 +/- ##
===========================================
+ Coverage 55.91% 56.00% +0.09%
===========================================
Files 1043 1046 +3
Lines 88934 89122 +188
===========================================
+ Hits 49729 49915 +186
- Misses 39205 39207 +2 ☔ View full report in Codecov by Sentry. |
…-circular-fileIO&common-dependancies
Just leaving a note here, no CI step is legitimately failing here. The CUDA builds were working but crash now because of a CI bug since I updated my branch. |
…IO&common-dependancies' into feature/rey/transmissibilityOutput
…om/GEOS-DEV/GEOS into feature/rey/transmissibilityOutput
This PR needs one more review. |
* added transmissibility unit * Added Cell-To-Cell output * Add the logLevel viewkey & description (cherrypick from feature/rey/sourceflux-stats) * Refactor the StencilOutput so the PackCollection can read it. Point is to allow to do an HDF output, and remove the CSV one. * Changing the Problem sub-groups init order so the outputs are at the end. Point is to solve a bug where the StencilDataCollection buffer size are not known for the HDF chunk sizes. * freeOnDevice fix test
* added transmissibility unit * Added Cell-To-Cell output * Add the logLevel viewkey & description (cherrypick from feature/rey/sourceflux-stats) * Refactor the StencilOutput so the PackCollection can read it. Point is to allow to do an HDF output, and remove the CSV one. * Changing the Problem sub-groups init order so the outputs are at the end. Point is to solve a bug where the StencilDataCollection buffer size are not known for the HDF chunk sizes. * freeOnDevice fix test
This PR is after the #3247.
This PR aims to add a new component: the
StencilOutput
, that allow to add aCellToCellOutput
task node in the xml to outputConnectionData
(global id pairs, transmissibilities).For now, by using the
writeCSV="1"
attribute, theCellToCellOutput
will output this data in a CSV. Here is an exemple:(aligned here for readability)