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

Scalar vel diff #116

Closed
wants to merge 13 commits into from
Closed

Conversation

dcoveney
Copy link
Contributor

@dcoveney dcoveney commented Dec 3, 2021

Created a switch, do_tensor_visc, that only applies diffuse_tensor_velocity() when the user has tensor viscosity - otherwise, diffuse_scalar() is used on the velocity components. Saves the computational time of calculating tensor cross terms when you know that they will be zero because of using scalar viscosity (Newtonian fluids).

@drummerdoc
Copy link
Contributor

A note of clarification: Tensor velocity solver is needed whenever the (scalar) viscosity has a spatial dependence (typically when it depends on state variables that have a spatial dependence).

@dcoveney
Copy link
Contributor Author

dcoveney commented Dec 3, 2021

Okay, thanks for pointing that out. So this should only be used in the case when you have a constant viscosity throughout the domain equal to vel_visc_coef?

@drummerdoc
Copy link
Contributor

Okay, thanks for pointing that out. So this should only be used in the case when you have a constant viscosity throughout the domain equal to vel_visc_coef?

I believe so....I worry about the word "only" though. I may not have a global enough view of the apps.

@asalmgren
Copy link
Contributor

asalmgren commented Dec 3, 2021 via email

@dcoveney
Copy link
Contributor Author

dcoveney commented Dec 3, 2021

In that case, I should probably set the default do_tensor_visc = 1 since that is the more general (and likely?) case.

for (int comp = 1; comp < nComp; comp++) {
if (a_is_diffusive[comp] != diffType) {
amrex::Abort("All the scalars must be either diffusive or non-diffusive when calling diffuse_scalar");
if(navier_stokes->do_tensor_visc){
Copy link
Collaborator

Choose a reason for hiding this comment

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

This if is not right. It's always required that all the scalars included here are either all diffusive or non-diffusive.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, I have taken that part out - is it okay now? The scalars you're sending to diffuse_scalar() are just the velocity components, so in any case if the velocity is not diffusive, all the components will not be diffusive and the function will exit.

Copy link
Collaborator

Choose a reason for hiding this comment

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

It would be good to add a check at the beginning of diffuse_scalar that ensures that velocity is done on it's own and not with scalars. The way beta is set requires this.

@cgilet
Copy link
Collaborator

cgilet commented Feb 9, 2022

ns.do_tensor_visc is not parsed from inputs. Please add that.

@cgilet
Copy link
Collaborator

cgilet commented Feb 9, 2022

It would also be good to have a check ensuring that we can only use this with divu=0

MultiFab** fluxn = fb_fluxn.get();
MultiFab** fluxnp1 = fb_fluxnp1.get();
const bool add_old_time_divFlux = true;
Vector<int> diffuse_comp(1);
Copy link
Collaborator

Choose a reason for hiding this comment

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

This needs to correspond to the member variable is_diffusive. We have to allow for the inviscid case. It's length also needs to match the number of components being diffused, as this is going to the general diffuse_scalar routine.

@drummerdoc
Copy link
Contributor

ns.do_tensor_visc is not parsed from inputs. Please add that.

Hopefully it will default to 1, true, ON, on, yes, YES, !0, or whatever :)

@cgilet
Copy link
Collaborator

cgilet commented Feb 9, 2022 via email

@asalmgren asalmgren closed this Jul 11, 2023
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.

None yet

5 participants