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

create Richards flow process #1473

Closed
wants to merge 39 commits into from
Closed

Conversation

Yonghui56
Copy link
Contributor

This PR includes:

  1. update the getDerivative function
  2. add getSupportMax() and getSupportMin() functions to return the max/min value of the input curve data.
  3. update the g-test for getDerivative function
  4. create the Richards flow process

@@ -326,6 +327,12 @@ void ProjectData::parseProcesses(BaseLib::ConfigTree const& processes_config,
"given dimension");
}
}
else if (type == "RICHARDS_FLOW")
{
process = ProcessLib::RichardsFlow::createRichardsFlowProcess(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tabs are forbidden. clang-format, please

@@ -22,6 +22,9 @@ APPEND_SOURCE_FILES(SOURCES TES)
add_subdirectory(HeatConduction)
APPEND_SOURCE_FILES(SOURCES HeatConduction)

add_subdirectory(RichardsFlow)
APPEND_SOURCE_FILES(SOURCES RichardsFlow)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line is sufficent. Remove the add_subdirectory(Richards) above and the empty CMakeLists.txt.

public:
LocalAssemblerData(MeshLib::Element const& element,
std::size_t const local_matrix_size,
bool is_axially_symmetric,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please add const qualifier (as for other arguments)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@endJunction you mean "bool const is_axially_symmetric" ??

Eigen::MatrixXd K_mat_coeff = Eigen::MatrixXd::Zero(1, 1);

MathLib::PiecewiseLinearInterpolation const& interP_Pc =
*_process_data.curves.at("curveA");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are not good names for curves. Be more precise.


MathLib::PiecewiseLinearInterpolation const& interP_Pc =
*_process_data.curves.at("curveA");
MathLib::PiecewiseLinearInterpolation const& interP_Kr =
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does inter stands for?

assert(local_matrix_size == ShapeFunction::NPOINTS * NUM_NODAL_DOF);

Eigen::MatrixXd mass_mat_coeff = Eigen::MatrixXd::Zero(1, 1);
Eigen::MatrixXd K_mat_coeff = Eigen::MatrixXd::Zero(1, 1);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move definition of the variables where they are used. There they can be const.

{
typename ShapeMatricesType::GlobalDimVectorType vec_g;
vec_g.resize(_dim);
vec_g(_dim - 1) = -9.81;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This quantity should be configurable from the input file.
See usage in HMFem code and creation in CreateHM and in prj file.

@endJunction
Copy link
Member

CTest(s) are missing.

@@ -72,29 +72,42 @@ double PiecewiseLinearInterpolation::getDerivative(

auto const& it(std::lower_bound(_supp_pnts.begin(), _supp_pnts.end(),
pnt_to_interpolate));
std::size_t const interval_idx = std::distance(_supp_pnts.begin(), it) - 1;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You do not subtract 1 here, but one every occurrence of interval_idx? What is the reason for this change? How is this related to the implementation of the Richards flow?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@TomFischer I did this because when "pnt_to_interpolate==_supp_pnts.begin()", std::distance(_supp_pnts.begin(), it) returns zero, further minus 1 will lead to a very large value...

Copy link
Member

@wenqing wenqing left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest to use the fluid and porous medium properties that have been already merged to the master. Since the Richards flow equation are very similar to the liquid flow equation, you can take my PR as a reference. Late on, relative permeability models and the capillary-saturation models, which include cure type models, can be also implemented.

@wenqing
Copy link
Member

wenqing commented Oct 11, 2016

@Yonghui56 Since there are still comments on my PR about the paring of material parameters, please wait a while.

@Yonghui56 Yonghui56 force-pushed the pr_richardsflow branch 2 times, most recently from c918d94 to f33c981 Compare October 11, 2016 11:32
@ogsbot
Copy link
Member

ogsbot commented Oct 11, 2016

Jenkins: OGS-6/Linux-PRs-dynamic failed: https://svn.ufz.de:8443/job/OGS-6/job/Linux-PRs-dynamic/1376/

@ogsbot
Copy link
Member

ogsbot commented Oct 11, 2016

Jenkins: OGS-6/Linux-PRs-dynamic failed: https://svn.ufz.de:8443/job/OGS-6/job/Linux-PRs-dynamic/1379/

@ogsbot
Copy link
Member

ogsbot commented Oct 11, 2016

Jenkins: OGS-6/Linux-PRs-dynamic failed: https://svn.ufz.de:8443/job/OGS-6/job/Linux-PRs-dynamic/1380/

Copy link
Member

@wenqing wenqing left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Material parameter stuff can be changed late on.

SecondaryVariableCollection&& secondary_variables,
NumLib::NamedFunctionCaller&& named_function_caller);

//! \name ODESystem interface
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this comment?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wenqing I just saw all the other processes hold this comment as well...

auto const rho_w = _process_data.water_density(t, pos)[0];
auto const body_force = _process_data.specific_body_force(t, pos);
assert(body_force.size() == GlobalDim);
auto const b =
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use MathLib::toVector as

 auto const& b = MathLib::toVector(body_force, GlobalDim);

with including #include "MathLib/LinAlg/Eigen/EigenMapTools.h"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wenqing I 've tried as you wrote, but it returns error message "no matching overloaded function"
if it goes like
auto const& b =
MathLib::toVectorEigen::VectorXd(body_force, GlobalDim);
it seems to be fine.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, copied extra characters in it. It is

auto const b = MathLib::toVector(body_force, GlobalDim);

@wenqing
Copy link
Member

wenqing commented Oct 12, 2016

Data conflict

@Yonghui56 Yonghui56 force-pushed the pr_richardsflow branch 2 times, most recently from 231268e to bd6e195 Compare October 13, 2016 09:27
@ogsbot
Copy link
Member

ogsbot commented Oct 18, 2016

Jenkins: OGS-6/Linux-PRs-dynamic failed: https://svn.ufz.de:8443/job/OGS-6/job/Linux-PRs-dynamic/1489/

sm.detJ * wp.getWeight();

local_b.noalias() += sm.dNdx.transpose() * K_mat_coeff * rho_w * b *
sm.detJ * wp.getWeight();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

integralMeasure

Copy link
Member

@endJunction endJunction left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've one more PR on your branch.
Please squash the changes to few commits and merge then, when the jenkins jobs are green.

Add back the has_gravity check.
auto const storage = _process_data.storage(t, pos)[0];
double const Pc = -P_int_pt;

double Sw = interpolated_Pc.getValue(Pc);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think Sw could be const? You could move line 115 to line 123.


_saturation[ip] = Sw;

double k_rel = interpolated_Kr.getValue(Sw);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could be const.

Copy link
Member

@wenqing wenqing left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It includes several files that does not belong to this PR. Maybe the problem was caused by improperly resolving conflicts after rebasing.

@@ -188,6 +188,28 @@ Eigen::Map<Vector> toVector(std::vector<double>& data,
return {data.data(), size};
}

/*! Creates an Eigen mapped vector from the given data vector.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only the format is changed in this file. Please use git checkout [ufz/master] MathLib/LinAlg/Eigen/EigenMapTools.h, and then use git add and git commit --amend to recovery it to its origin.


// copy shape matrix to extrapolation matrix columnwise
N.block(int_pt, 0, 1, nn) = shp_mat;
auto const pair_it_inserted = _qr_decomposition_cache.emplace(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file is just the same as that in ufz/master. Please recovery it.

std::vector<double> _integration_point_values_cache;

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file is not changed either.

@@ -15,22 +15,6 @@
#include "NeumannBoundaryCondition.h"
#include "RobinBoundaryCondition.h"

static std::vector<MeshLib::Element*> getClonedElements(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure whether these changes from this PR.

}
else
{
OGS_FATAL("Unknown boundary condition type: `%s'.", type.c_str());
}
}

std::unique_ptr<BoundaryCondition>
BoundaryConditionBuilder::createDirichletBoundaryCondition(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure whether these changes from this PR.

@@ -12,11 +12,23 @@

#include "NumLib/NumericsConfig.h"

namespace GeoLib
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure whether the changes in this file from this PR.

@@ -17,6 +17,7 @@ APPEND_SOURCE_FILES(SOURCES GroundwaterFlow)
APPEND_SOURCE_FILES(SOURCES SmallDeformation)

APPEND_SOURCE_FILES(SOURCES SmallDeformationWithLIE)
APPEND_SOURCE_FILES(SOURCES SmallDeformationWithLIE/BoundaryCondition)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change should not be in this PR. Sounds like a rebasing problem.

@@ -11,7 +11,6 @@
#define PROCESS_LIB_GROUNDWATERFLOWPROCESS_H_

#include "NumLib/DOF/LocalToGlobalIndexMap.h"
#include "NumLib/Extrapolation/LocalLinearLeastSquaresExtrapolator.h"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From other merged PR.

@@ -10,7 +10,6 @@
#ifndef PROCESS_LIB_HEATCONDUCTIONPROCESS_H_
#define PROCESS_LIB_HEATCONDUCTIONPROCESS_H_

#include "NumLib/Extrapolation/LocalLinearLeastSquaresExtrapolator.h"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From other merged PR.

@wenqing
Copy link
Member

wenqing commented Oct 19, 2016

Younghui found the reason: the extra files in this PR are from the wrong ufz/master.

double k_rel = interpolated_Kr.getValue(Sw);
double drhow_dp(0.0);
double const k_rel = interpolated_Kr.getValue(Sw);
double const drhow_dp(0.0);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Multiplying by 'drhow_dp, which is zero and const, make 'mass_mass_coeff 0. Is this intended?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commented in the code. But only part of the expression below would be zero. The other things still contribute.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not see where this is commented in the code, but you are right, the mass_mat_coeff is not zero. I am unsure how the compiler handles the expression poro * Sw * drhow_dp and how important the optimization of the calculation of this expression is.

@endJunction
Copy link
Member

Merged.

@ogsbot
Copy link
Member

ogsbot commented Jun 19, 2020

OpenGeoSys development has been moved to GitLab.

See this pull request on GitLab.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants