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

Ideal Gas Law property for generalized material property lib #2651

Merged
merged 7 commits into from Sep 9, 2019

Conversation

@Scinopode
Copy link
Contributor

Scinopode commented Sep 5, 2019

Implementation for entire phases, an expansion for multiple components is coming soon.

  1. Feature description was added to the changelog
  2. Tests covering your feature were added?
  3. Any new feature or behavior change was documented?

namespace MaterialPropertyLib
{
IdealGasLaw::IdealGasLaw(){};

This comment has been minimized.

Copy link
@TomFischer

TomFischer Sep 5, 2019

Member

The compiler will generate the constructor for you. You can remove the lines.

{
IdealGasLaw::IdealGasLaw(){};

/**

This comment has been minimized.

Copy link
@TomFischer

TomFischer Sep 5, 2019

Member

You can remove the comment signs when there isn't any comment.

* \brief
*
* \copyright
* Copyright (c) 2012-2018, OpenGeoSys Community (http://www.opengeosys.org)

This comment has been minimized.

Copy link
@TomFischer

TomFischer Sep 5, 2019

Member
Suggested change
* Copyright (c) 2012-2018, OpenGeoSys Community (http://www.opengeosys.org)
* Copyright (c) 2012-2019, OpenGeoSys Community (http://www.opengeosys.org)
* \brief
*
* \copyright
* Copyright (c) 2012-2018, OpenGeoSys Community (http://www.opengeosys.org)

This comment has been minimized.

Copy link
@TomFischer

TomFischer Sep 5, 2019

Member
Suggested change
* Copyright (c) 2012-2018, OpenGeoSys Community (http://www.opengeosys.org)
* Copyright (c) 2012-2019, OpenGeoSys Community (http://www.opengeosys.org)
@wenqing
wenqing approved these changes Sep 6, 2019
Copy link
Member

wenqing left a comment

Looks good except one code duplication

double const t) const
{
double molar_mass = 0.0;
if (_phase) // IdealGasLaw of an entire phase

This comment has been minimized.

Copy link
@wenqing

wenqing Sep 6, 2019

Member

To avoid the code duplication, you set a local function to get molar_mass.

return -pressure * molar_mass / gas_constant / temperature /
temperature;
}
else if (primary_variable == Variable::phase_pressure)

This comment has been minimized.

Copy link
@endJunction

endJunction Sep 6, 2019

Member

no else after return...

@Scinopode Scinopode force-pushed the Scinopode:mpl_ideal_gas_law branch from ee057c8 to b3efe1f Sep 9, 2019
@Scinopode Scinopode force-pushed the Scinopode:mpl_ideal_gas_law branch from b3efe1f to de9a1a2 Sep 9, 2019
Copy link
Member

TomFischer left a comment

When test green:

@TomFischer TomFischer merged commit 2c42abd into ufz:master Sep 9, 2019
3 checks passed
3 checks passed
continuous-integration/jenkins/pr-merge This commit looks good
Details
deploy/netlify Deploy preview ready!
Details
ufz.ogs #20190909.2 succeeded
Details
@Scinopode Scinopode deleted the Scinopode:mpl_ideal_gas_law branch Sep 11, 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.