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

[MPL] Add Property based on Parameter. #2616

Merged
merged 10 commits into from Aug 20, 2019
Merged

Conversation

@TomFischer
Copy link
Member

TomFischer commented Aug 19, 2019

PR implements a new Property based on Parameter. With this implementation spatial heterogeneous properties are available now.

  1. Feature description was added to the changelog
  2. Tests covering your feature were added?
    The disabled HT decovalex test was re-enable.
  3. Any new feature or behavior change was documented?
@renchao-lu

This comment has been minimized.

Copy link
Member

renchao-lu commented Aug 19, 2019

So great!

Copy link
Member

wenqing left a comment

👍

const double density)
double getThermalExpansivity(Phase const& phase, VariableArray const& vars,
const double density,
ParameterLib::SpatialPosition const& pos, double t)

This comment has been minimized.

Copy link
@endJunction

endJunction Aug 19, 2019

Member
Suggested change
ParameterLib::SpatialPosition const& pos, double t)
ParameterLib::SpatialPosition const& pos, double const t)
@@ -170,6 +173,17 @@ std::unique_ptr<MaterialPropertyLib::Property> createProperty(
reference_value, exp_data);
}

if (property_type == "parameter")

This comment has been minimized.

Copy link
@endJunction

endJunction Aug 19, 2019

Member

parameter or Parameter? If the former, then in line 179 it should be

//! \ogs_file_param{properties__property__parameter__parameter_name}
PropertyDataType value(VariableArray const& variable_array) const override;
PropertyDataType value(VariableArray const& variable_array,
ParameterLib::SpatialPosition const& pos,
double t) const override;

This comment has been minimized.

Copy link
@endJunction

endJunction Aug 19, 2019

Member
Suggested change
double t) const override;
double const t) const override;
PropertyDataType ParameterProperty::value(
VariableArray const& /*variable_array*/,
ParameterLib::SpatialPosition const& pos,
double t) const

This comment has been minimized.

Copy link
@endJunction

endJunction Aug 19, 2019

Member
Suggested change
double t) const
double const t) const
T value(VariableArray const& variable_array) const
T value(VariableArray const& variable_array,
ParameterLib::SpatialPosition const& pos,
double t) const

This comment has been minimized.

Copy link
@endJunction

endJunction Aug 19, 2019

Member
Suggested change
double t) const
double const t) const
const double specific_heat_capacity_fluid)
MaterialPropertyLib::VariableArray const& vars, const double porosity,
const double fluid_density, const double specific_heat_capacity_fluid,
ParameterLib::SpatialPosition const& pos, double t)

This comment has been minimized.

Copy link
@endJunction

endJunction Aug 19, 2019

Member
Suggested change
ParameterLib::SpatialPosition const& pos, double t)
ParameterLib::SpatialPosition const& pos, double const t)
<property>
<name>density</name>
<type>parameter</type>
<parameter_name>rho_solid</parameter_name>

This comment has been minimized.

Copy link
@endJunction

endJunction Aug 19, 2019

Member

For discussion:
Could be also

<property>
    <name>density</name>
    <parameter>rho_solid</parameter>
</property>

What do you think?
@TomFischer I know, the parsing is slightly more difficult.

This comment has been minimized.

Copy link
@TomFischer

TomFischer Aug 20, 2019

Author Member

The advantage of your variant is a non-redundant, short description of a ParameterProperty object. The advantage of the implementation of the PR is that it is consistent with the specification of other parameter types.

@wenqing, @renchao-lu, @ErikNixdorf , @waltherm Opinions?

This comment has been minimized.

Copy link
@wenqing

wenqing Aug 20, 2019

Member

I think that it is better to have the consistency with other types of input. Therefore I prefer to take the present one in this PR.

This comment has been minimized.

Copy link
@renchao-lu

renchao-lu Aug 20, 2019

Member

In my mind, there are three types of property offered by far as follows: constant, linear dependency, and exponential dependency. I guess the initial design of the tag <type> intends to establish property dependency linking to primary process variables which will change over time. If a material property is not time-dependent, we assign the tag to constant. Otherwise, we assign to linear/exponential dependency. As to ParameterLib, I think the major business of ParameterLib is to tackle spatial heterogeneity, although it offers some time-associated functionalities. For this main reason, ParameterLib has its own tag for property type: function, group, meshnode, and meshelement (for spatial heterogeneity), curvescaled, and timedependentheterogeneousparameter (for time-dependency).

This comment has been minimized.

Copy link
@waltherm

waltherm Aug 20, 2019

Contributor

I would vote for @TomFischer implementation, which seems more consistent with the existing, alternative options.

This comment has been minimized.

Copy link
@endJunction

endJunction Aug 20, 2019

Member

Thanks for discussion.

@TomFischer TomFischer force-pushed the TomFischer:AddParameterToMPL branch 3 times, most recently from 0651908 to a84cdce Aug 20, 2019
@TomFischer TomFischer force-pushed the TomFischer:AddParameterToMPL branch from a84cdce to c626c06 Aug 20, 2019
@@ -199,7 +198,8 @@ class HTFEM : public HTLocalAssemblerInterface
GlobalDimMatrixType getThermalConductivityDispersivity(
MaterialPropertyLib::VariableArray const& vars, const double porosity,
const double fluid_density, const double specific_heat_capacity_fluid,
const GlobalDimVectorType& velocity, const GlobalDimMatrixType& I)
const GlobalDimVectorType& velocity, const GlobalDimMatrixType& I,
ParameterLib::SpatialPosition const& pos, double t)

This comment has been minimized.

Copy link
@endJunction

endJunction Aug 20, 2019

Member
Suggested change
ParameterLib::SpatialPosition const& pos, double t)
ParameterLib::SpatialPosition const& pos, double const t)
@TomFischer TomFischer force-pushed the TomFischer:AddParameterToMPL branch from c626c06 to 4ea7df2 Aug 20, 2019
@bilke bilke merged commit ef0ca26 into ufz:master Aug 20, 2019
2 of 3 checks passed
2 of 3 checks passed
ufz.ogs #20190820.8 failed
Details
continuous-integration/jenkins/pr-merge This commit looks good
Details
deploy/netlify Deploy preview ready!
Details
@TomFischer TomFischer deleted the TomFischer:AddParameterToMPL branch Aug 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.