search

GEOS-DEV / GEOS

GEOS Simulation Framework
GNU Lesser General Public License v2.1
203 stars 80 forks source link

Register pressureGradient only for hybrid FVM (single phase) #3125

Closed paveltomin closed 1 month ago

codecov[bot] commented 1 month ago

Codecov Report

Attention: Patch coverage is 20.00000% with 8 lines in your changes are missing coverage. Please review.

Project coverage is 53.56%. Comparing base (7ace2fc) to head (00d0d04).

Files Patch % Lines
.../physicsSolvers/fluidFlow/SinglePhaseHybridFVM.cpp 0.00% 8 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## develop #3125 +/- ## =========================================== - Coverage 53.57% 53.56% -0.01% =========================================== Files 1003 1003 Lines 85289 85297 +8 =========================================== - Hits 45696 45693 -3 - Misses 39593 39604 +11 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

CusiniM commented 1 month ago

In reality, it would be good if we could find a way to compute this also for tpfa. It's just harder to come up with a way to approximate it.

paveltomin commented 1 month ago

In reality, it would be good if we could find a way to compute this also for tpfa. It's just harder to come up with a way to approximate it.

something like that? image @karimifard can confirm if that makes sense

CusiniM commented 1 month ago

In reality, it would be good if we could find a way to compute this also for tpfa. It's just harder to come up with a way to approximate it.

something like that? image @karimifard can confirm if that makes sense

yeah, what's annoying is the number of derivatives that entails... I guess we will just lag the term.

karimifard commented 1 month ago

In reality, it would be good if we could find a way to compute this also for tpfa. It's just harder to come up with a way to approximate it.

something like that? image @karimifard can confirm if that makes sense

yeah, what's annoying is the number of derivatives that entails... I guess we will just lag the term.

The formula should work for Cartesian grids but it's not clear to me it will work for non Cartesian unstructured grids. If I am not wrong, it is essentially a weighted average of face velocity, with the weights being distance to face times the face area, which is a volume. In the Cartesian case, these weights add up to the volume and the formula works but in general adding all the weights/volumes will be larger than the volume of the cell. So if we want to use something like (17) we need to adjust the cell volume, or maybe just average the face velocities (q/A). Another way to estimate the velocity inside the element is to use a least square approximation. I may have missed something...

CusiniM commented 1 month ago

In reality, it would be good if we could find a way to compute this also for tpfa. It's just harder to come up with a way to approximate it.

something like that? image @karimifard can confirm if that makes sense

yeah, what's annoying is the number of derivatives that entails... I guess we will just lag the term.

The formula should work for Cartesian grids but it's not clear to me it will work for non Cartesian unstructured grids. If I am not wrong, it is essentially a weighted average of face velocity, with the weights being distance to face times the face area, which is a volume. In the Cartesian case, these weights add up to the volume and the formula works but in general adding all the weights/volumes will be larger than the volume of the cell. So if we want to use something like (17) we need to adjust the cell volume, or maybe just average the face velocities (q/A). Another way to estimate the velocity inside the element is to use a least square approximation. I may have missed something...

The least square approximation s what we use for the Mimetic. the annoying thing with tpfa was that we don't store the face velocities so to compute cell center values we would need to do another stencil loop coz we need the neighbors' pressures.