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

wall model refactor #335

Merged
merged 4 commits into from
Feb 8, 2021
Merged

wall model refactor #335

merged 4 commits into from
Feb 8, 2021

Conversation

michaeljbrazell
Copy link
Contributor

@michaeljbrazell michaeljbrazell commented Feb 5, 2021

  • added more wall models
  • I'm not fully confident in any of the non Moeng heat flux wall model implementations
  • @jrood-nrel this will cause a small diff for abl_stable due to order of operations: taux * den * utau2 vs. taux * utau2 * den

@@ -266,12 +267,19 @@ void ABLWallFunction::computeusingheatflux()

ABLVelWallFunc::ABLVelWallFunc(Field&, const ABLWallFunction& wall_func)
: m_wall_func(wall_func)
{}
{
amrex::ParmParse pp("ShearStress");
Copy link
Contributor

Choose a reason for hiding this comment

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

I would prefer not adding another namespace here. Instead use ABL.wall_shear_stress_type = moeng ?

@@ -349,11 +379,17 @@ void ABLVelWallFunc::operator()(Field& velocity, const FieldState rho_state)

ABLTempWallFunc::ABLTempWallFunc(Field&, const ABLWallFunction& wall_fuc)
: m_wall_func(wall_fuc)
{}
{
amrex::ParmParse pp("HeatFlux");
Copy link
Contributor

Choose a reason for hiding this comment

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

Can reuse ABL.wall_shear_stress_type here? I don't think that we have a use-case where we would do things differently for vel/temperature, is there? So one input parameter would suffice?

Comment on lines 451 to 452
}
void ABLTempWallFunc::operator()(Field& temperature, const FieldState rho_state)
Copy link
Contributor

Choose a reason for hiding this comment

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

clang-format hopefully complained here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it missed it!

Comment on lines 478 to 480
} else {
amrex::Abort("Shear Stress temperature wall model input mistake");
}
Copy link
Contributor

Choose a reason for hiding this comment

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

We should check this during input parsing time and remove this else.

Comment on lines 1 to 6
//
// ShearStress.H
// amr-wind
//
//

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
//
// ShearStress.H
// amr-wind
//
//

term1 = mo.utau * mo.kappa / mo.phi_h();
}

AMREX_GPU_HOST_DEVICE amrex::Real
Copy link
Contributor

Choose a reason for hiding this comment

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

We should also add AMREX_DEVICE_FORCE_INLINE


struct ShearStressConstant
{
ShearStressConstant(amr_wind::MOData mo)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
ShearStressConstant(amr_wind::MOData mo)
ShearStressConstant(const amr_wind::MOData& mo)

avoid a copy here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also should declare these constructors explicit

, theta_mean(mo.theta_mean)
, theta_surface(mo.surf_temp)
{
term1 = mo.utau * mo.kappa / mo.phi_h();
Copy link
Contributor

Choose a reason for hiding this comment

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

This can be initialized in the constructor initialization list also.

amrex::Real term1;
};

struct ShearStressMoeng
Copy link
Contributor

Choose a reason for hiding this comment

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

May be add doxygen docs and assign them to ABL group?

Copy link
Contributor

@gantech gantech left a comment

Choose a reason for hiding this comment

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

This is fantastic!

@sayerhs sayerhs merged commit 19faed0 into Exawind:main Feb 8, 2021
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.

3 participants