Skip to content
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

Dupuit #2330

Merged
merged 12 commits into from Aug 2, 2019

Conversation

@TomFischer
Copy link
Member

commented Jan 23, 2019

Implementation of Dupuit permeability relation.

  • Changelog entry
  • documentation
  • tests / benchmarks

Update
Later PR:

  • storage model

@TomFischer TomFischer added the WIP 🏗 label Jan 23, 2019

@codecov

This comment has been minimized.

Copy link

commented Feb 4, 2019

Codecov Report

Merging #2330 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2330   +/-   ##
=======================================
  Coverage   32.21%   32.21%           
=======================================
  Files         574      574           
  Lines       21246    21246           
  Branches     9984     9984           
=======================================
  Hits         6844     6844           
  Misses      10916    10916           
  Partials     3486     3486

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6070daf...9a2a0c5. Read the comment docs.

@TomFischer TomFischer force-pushed the TomFischer:Dupuit branch from 4bba458 to 1eee440 May 14, 2019

@TomFischer

This comment has been minimized.

Copy link
Member Author

commented May 14, 2019

2019-05-14: rebased

@TomFischer TomFischer force-pushed the TomFischer:Dupuit branch from 1eee440 to 2b5e6e4 Jun 5, 2019

@TomFischer TomFischer force-pushed the TomFischer:Dupuit branch from 2b5e6e4 to 9a2a0c5 Jun 13, 2019

@Thomas-TK

This comment has been minimized.

Copy link
Member

commented Jun 18, 2019

@TomFischer

  • documentation and 4 benchmark tests are prepared.
    We would need now a place to upload them. I would call the headline "Dupuit assumption" or "unconfined groundwater flow".
@Thomas-TK

This comment has been minimized.

Copy link
Member

commented Jun 18, 2019

Just wondering while looking at the benchmark documentation: if we have a headline called elliptic, why don't we have parabolic as well?

@Thomas-TK

This comment has been minimized.

Copy link
Member

commented Jun 18, 2019

. . I guess I found the answer: web\content\docs\benchmarks. To be prepared with markdown. The web-folder contains a readme :-)

@TomFischer TomFischer force-pushed the TomFischer:Dupuit branch 2 times, most recently from c14a36d to 960ed1c Jul 11, 2019

@TomFischer TomFischer added please review and removed WIP 🏗 labels Jul 12, 2019

const double variable,
const double temperature) const override
{
_intrinsic_permeability_tensor =

This comment has been minimized.

Copy link
@endJunction

endJunction Jul 26, 2019

Member

Why storing a value which is recomputed every time?

Suggested change
_intrinsic_permeability_tensor =
return

This comment has been minimized.

Copy link
@endJunction

This comment has been minimized.

Copy link
@TomFischer

TomFischer Jul 30, 2019

Author Member

Tried the suggested change. It crashes the benchmarks. Don't know why.

This comment has been minimized.

Copy link
@TomFischer

TomFischer Jul 31, 2019

Author Member

Removing _intrinsic_permeability_tensor leads to the compiler warning:

 Permeability.h:60:16: warning: returning reference to local temporary object [-Wreturn-stack-address]
        return Eigen::Map<Eigen::Matrix<double, Eigen::Dynamic, Eigen::Dynamic,

@TomFischer TomFischer force-pushed the TomFischer:Dupuit branch 2 times, most recently from abd0c62 to c112453 Jul 29, 2019

@@ -68,7 +69,7 @@ class Permeability
return _intrinsic_permeability_tensor;
}

private:
protected:
ParameterLib::Parameter<double> const& _permeability_parameter;
int const _dimension;
mutable Eigen::MatrixXd _intrinsic_permeability_tensor;

This comment has been minimized.

Copy link
@endJunction

endJunction Jul 30, 2019

Member

Try to remove this member. I don't see where it is used. In two getValue functions the tensor is computed every time.

@TomFischer TomFischer force-pushed the TomFischer:Dupuit branch from c112453 to 03e12c1 Jul 30, 2019

@endJunction endJunction force-pushed the TomFischer:Dupuit branch from a8fa33b to d17cd6f Jul 31, 2019

@TomFischer TomFischer force-pushed the TomFischer:Dupuit branch 2 times, most recently from 9135cea to 67b2ffd Jul 31, 2019

@wenqing
Copy link
Member

left a comment

I think there is no need to change the permeability model.

The problem is a special case of groundwater problem, which has large horizontal spatial size and is simplified as 2D problem. Theoretically, the hydraulic head has to be used as primary variable. One can use the pore pressure as the primary variable but the gravity term can not be ignored because
h = H-z, where H is the pressure head given by p/(rho*g), and z is the relative vertical coordinate.
Besides, grad (K h grad h) gives high non-linearity, its weak form can be expanded as <K h grad h, u>_b - <K h grad h, grad u>_D. This expression says that it is not adequate to add a new permeability model.

Based on the above points, I suggest to derived a class from the local assembler of GroundwaterFlow to assemble Jacobian for the problem, and at the end solve the problem by the Newton-Raphson method.

@wenqing

This comment has been minimized.

Copy link
Member

commented Aug 1, 2019

One can set mu=1, rho=0 in the LiquidFlow to use pressure as head. Then one has to keep in mind that the permeability variable keeps the value of hydraulic conductivity. This would cause some sort of confusion.

@endJunction

This comment has been minimized.

Copy link
Member

commented Aug 1, 2019

@wenqing Would this also be applicable to HT, for example? As I understood it, the "permeability model" can be used in other processes too.

@wenqing

This comment has been minimized.

Copy link
Member

commented Aug 1, 2019

@endJunction I think it is only for a groundwater model. As far as I could understand that this PR wants to handle hK in \beta h = grad (h K grad h) for large spatial groundwater problem by introducing a permeability model of hK. You can see h K cannot be considered as hydraulic conductivity. The problem is just how to deal with unknowns. Furthermore the nonlinearity of grad (h K grad h) has to be dealt properly (better use Newton-Raphson) for the solution accuracy.

@wenqing

This comment has been minimized.

Copy link
Member

commented Aug 1, 2019

I have checked that the implementation of the unconfined equation is correct with the Picard non-linear solver. As discussed that there should be explanation in the documentation of DupuitPermeability that DupuitPermeability provides h *K for unconfined flow equation, and its name is meaningless for such case.

TomFischer and others added some commits Jul 31, 2019

Permeability; Return matrix by value.
This avoids the mutable member. The data is computed
on every call anyway.

@TomFischer TomFischer force-pushed the TomFischer:Dupuit branch from 67b2ffd to 113b28c Aug 2, 2019

@TomFischer

This comment has been minimized.

Copy link
Member Author

commented Aug 2, 2019

@wenqing Please, could you check the changed documentation of DupuitPermeability class.

@wenqing

wenqing approved these changes Aug 2, 2019

{
/// The purpose of the class DupuitPermeability is the implementation of the
/// special diffusion coefficient \f$h K(h)\f$, where \f$h\f$ is the hydraulic
/// head and \f$K\f$ is the hydraulic conductivity. The diffusion coefficient is

This comment has been minimized.

Copy link
@wenqing

wenqing Aug 2, 2019

Member

... the hydraulic head and ... -->... the hydraulic head of the previous non-linear iteration and ... ✔️

@TomFischer TomFischer force-pushed the TomFischer:Dupuit branch from 113b28c to 6ca337f Aug 2, 2019

@TomFischer TomFischer merged commit 8800c64 into ufz:master Aug 2, 2019

1 of 3 checks passed

continuous-integration/jenkins/pr-merge This commit cannot be built
Details
ufz.ogs in progress
Details
deploy/netlify Deploy preview ready!
Details

@TomFischer TomFischer deleted the TomFischer:Dupuit branch Aug 2, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.