GEOS-DEV / GEOS

GEOS Simulation Framework
GNU Lesser General Public License v2.1
210 stars 84 forks source link

refactor: fix creation of mass handling. #3215

Closed CusiniM closed 2 months ago

CusiniM commented 3 months ago

This PR simply moves the location of the assignment of the createdMass variable that tracks how much mass is created after each propagation step. This allows to set its default values to zero so taht we don't see artifacts in simulations that do not include any propagation and for which that variable is useless. Personally, I think that the initialization of new elements and how that createdMass is used should be revisited. For example, it should be equal to mass_n for a new element and currently it is not.

However, it is easier do that separately because it will impact the solution of the hydraulic fracturing tests so I propose that we start by merging this simple change and address the actual algorithm later.

ryar9534 commented 3 months ago

Does this interact at all with #3194?

rrsettgast commented 3 months ago

@CusiniM I remember the reason that we can't get rid of the creationMass is because there isn't anywhere to steal it from in the case of a viscosity dominated case with no pore fluid in the rock. For example, in a KGD when creating a new face (new tip), the old tip has a negative pressure and taking mass from it will mess up the simulation. We really need to implement some concept of partial fracture or saturation at the tip to avoid this.

In the case where there is pore fluid, we should steal it from the rock pores. If there is no pore fluid...then there is no good answer except for a partial fracture/saturation treatment that I can think of

paveltomin commented 3 months ago

Does this interact at all with #3194?

It should not, I reverted everything related to mass creation in that PR

CusiniM commented 2 months ago

@CusiniM I remember the reason that we can't get rid of the creationMass is because there isn't anywhere to steal it from in the case of a viscosity dominated case with no pore fluid in the rock. For example, in a KGD when creating a new face (new tip), the old tip has a negative pressure and taking mass from it will mess up the simulation. We really need to implement some concept of partial fracture or saturation at the tip to avoid this.

In the case where there is pore fluid, we should steal it from the rock pores. If there is no pore fluid...then there is no good answer except for a partial fracture/saturation treatment that I can think of

Yeah, I have not changed anything of the logic of how it's used. I have only changed where it's initialized.

I still think that, since we use an implicit scheme, mass_n should be 0 for a newly created element. This would make the scheme conservative. I do remember that we struggle with convergence though. The good news is that, in reality, the case with no pore fluid in the rock is only useful to match analytical solutions because all real applications should consider a saturated rock around it.

codecov[bot] commented 2 months ago

Codecov Report

Attention: Patch coverage is 1.19048% with 83 lines in your changes missing coverage. Please review.

Project coverage is 55.83%. Comparing base (b7e96d1) to head (504d6ba).

Files Patch % Lines
...hysicsSolvers/multiphysics/HydrofractureSolver.cpp 0.00% 76 Missing :warning:
...nents/physicsSolvers/fluidFlow/SinglePhaseBase.cpp 20.00% 4 Missing :warning:
.../physicsSolvers/fluidFlow/FlowSolverBaseFields.hpp 0.00% 1 Missing :warning:
...hysicsSolvers/fluidFlow/SinglePhaseBaseKernels.hpp 0.00% 1 Missing :warning:
...olvers/fluidFlow/ThermalSinglePhaseBaseKernels.hpp 0.00% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## develop #3215 +/- ## =========================================== - Coverage 55.83% 55.83% -0.01% =========================================== Files 1042 1042 Lines 88653 88654 +1 =========================================== Hits 49501 49501 - Misses 39152 39153 +1 ```

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