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

Coupling FE Scalar Field Variable to FV Kernels #29106

Open
wants to merge 6 commits into
base: next
Choose a base branch
from

Conversation

csdechant
Copy link

@csdechant csdechant commented Nov 20, 2024

Reason

This PR will enable the coupling of FE scalar field variable values and gradients to FV kernels. This PR directly addresses issue #15062

Design

This PR includes the following:

  • Introduces adCoupledGradientFace, which returns gradient of a coupled variable at an element face for FV Kernel.
  • Tests the coupling of FE scalar field variable values to FVElementKernel and FVFluxKernel.
  • Tests the coupling of FE scalar field variable gradients to FVElementKernel and FVFluxKernel.
  • Tests the coupling of FV variable gradients to FVElementKernel and FVFluxKernel.
  • Introduces convergence studies for all new kernel objects as heavy tests.

Impact

This PR will allow for great degree of user flexibility when solving multiphysics problems where some physics are favorable towards FE and other physics are favorable towards FE (e.g. coupling FE electromagnetics to FV conducting fluids).

@moosebuild
Copy link
Contributor

moosebuild commented Nov 20, 2024

Job Documentation, step Docs: sync website on a947630 wanted to post the following:

View the site here

This comment will be updated on new commits.

@lindsayad
Copy link
Member

when you say scalar, you mean scalar field right? Because MOOSE has scalar variables whose values are independent of position on the mesh

@csdechant csdechant changed the title Coupling FE Scalar Variable to FV Kernels Coupling FE Scalar Field Variable to FV Kernels Nov 20, 2024
@csdechant
Copy link
Author

csdechant commented Nov 20, 2024

@lindsayad you're absolutely correct. I use the term scalar to denote that vector variables (like Nédélec variables) are not considered in this PR, but I forgot about MOOSE's Scalar variable and kernel objects. I renamed the PR to clarify scalar field.

@lindsayad
Copy link
Member

I figured but just wanted to check. I'll start reviewing once more tests are passing

Copy link
Contributor

@GiudGiud GiudGiud left a comment

Choose a reason for hiding this comment

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

this looks promising
can you add a section in the Coupleable.md (most likely?) on coupling FE variables in FV objects

@@ -283,6 +283,12 @@ class Assembly
*/
const QBase * const & qRuleFace() const { return constify_ref(_current_qrule_face); }

/**
* Returns the reference to the current FV quadrature being used on a current face
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
* Returns the reference to the current FV quadrature being used on a current face
* Returns the reference to a pointer to the current FV quadrature being used on a current face

@@ -691,7 +697,7 @@ class Assembly
*/
void reinit(const Elem * elem, unsigned int side, const std::vector<Point> & reference_points);

void reinitFVFace(const FaceInfo & fi);
void reinitFVFace(const FaceInfo & fi, bool areFE);
Copy link
Contributor

Choose a reason for hiding this comment

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

docstring, what does areFE mean

@@ -630,6 +630,17 @@ class Coupleable
virtual const ArrayVariableValue & coupledArrayValueOlder(const std::string & var_name,
unsigned int comp = 0) const;

/**
* Returns gradient of a coupled variable at an element face for FV Kernel / BC coupling
Copy link
Contributor

Choose a reason for hiding this comment

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

ideally coupleable does not mention "FV" as all routines would work for all variables? (just need to be implemented)

Comment on lines +132 to +133
/// Whether the MooseObject is a finite volume object
const bool _is_fv;
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
/// Whether the MooseObject is a finite volume object
const bool _is_fv;

use the Coupleable is_fv attribute (make it protected if it s not)

// FE coupled variables. If there are some FE variables, then FE reinit is
// necessary.
bool areFE = false;
for (auto var : _fv_vars)
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
for (auto var : _fv_vars)
for (auto var : _vars)

}

// NOTES: GhostValues for FE variable with equal the Face average value
// of the neighbor side.
Copy link
Contributor

Choose a reason for hiding this comment

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

let's move this inside the routine

_element_data->computeGhostValuesFace(fi, *_neighbor_data);
}
else
mooseError("FE to FV coupling broken.");
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
mooseError("FE to FV coupling broken.");
mooseAssert(false, "FE to FV coupling broken.");

We dont expect to hit this in production

void
MooseVariableFE<OutputType>::computeAdGradFaceAvg(const FaceInfo & /*fi*/)
{
mooseError("computeAdGradFaceAvg(const FaceInfo & fi) are only for MooseVariableFE<Real>.");
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
mooseError("computeAdGradFaceAvg(const FaceInfo & fi) are only for MooseVariableFE<Real>.");
mooseError("computeAdGradFaceAvg(const FaceInfo & fi) are only implemented for MooseVariableFE<Real>.");

VectorValue<ADReal>
MooseVariableFE<OutputType>::adGradSln(const FaceInfo & /*fi*/, const Moose::StateArg & /*state*/)
{
mooseError("adGradSln(const FaceInfo & fi) are only for MooseVariableFE<Real>.");
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
mooseError("adGradSln(const FaceInfo & fi) are only for MooseVariableFE<Real>.");
mooseError("adGradSln(const FaceInfo & fi) are only implemented for MooseVariableFE<Real>.");


_ad_grad_u_face = adGradSlnAvg();
_ad_grad_neighbor_u_face = adGradSlnAvgNeighbor();
_ad_grad_face_avg = (_ad_grad_u_face[0] + _ad_grad_neighbor_u_face[0]) / 2.0;
Copy link
Contributor

Choose a reason for hiding this comment

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

this is for a FE variable right

can you think of a reason to not have equal weights?

_ad_zero = 0;
unsigned int nqp = _current_qrule->n_points();
// NOTES: This seems like a lot of if statements. Maybe there is better way
// to seperate face vs element averaging...
Copy link
Contributor

Choose a reason for hiding this comment

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

you could split it in computeADElementAveraging and computeADFaceAveraging

void
MooseVariableData<OutputType>::computeADAveraging()
{
_ad_zero = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

do we need this?

Comment on lines +2476 to 2478
/// FV quadrature rule used on faces
QBase * _current_FV_qrule_face;
/// The current arbitrary quadrature rule used on element faces
Copy link
Contributor

Choose a reason for hiding this comment

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

if there is another solution here than doubling up the attribute, it'd be good

@lindsayad lindsayad self-assigned this Nov 22, 2024
@GiudGiud
Copy link
Contributor

GiudGiud commented Nov 23, 2024

Maybe some high level questions,

  • could we then use any routine from Coupleable in a FVKernel and get the expected computations or do we have to pick only the new ones (for elemental and flux kernels respectively). If the latter is there a way to enforce this?
  • Why is the average value preferred over computing the centroid value? Is there some theory around this? I guess for the volume terms this leads to a conservative coupling. What about faces?

@csdechant csdechant force-pushed the FEtoFVCoupling-Rebased branch from 8bc7e25 to d1db6ab Compare December 4, 2024 21:00
@moosebuild
Copy link
Contributor

Job Precheck, step Clang format on d1db6ab wanted to post the following:

Your code requires style changes.

A patch was auto generated and copied here
You can directly apply the patch by running, in the top level of your repository:

curl -s https://mooseframework.inl.gov/docs/PRs/29106/clang_format/style.patch | git apply -v

Alternatively, with your repository up to date and in the top level of your repository:

git clang-format 586fdf59d3f0efb469a3717233db6d0ed2634848

@moosebuild
Copy link
Contributor

Job Coverage, step Generate coverage on a947630 wanted to post the following:

Framework coverage

586fdf #29106 a94763
Total Total +/- New
Rate 85.15% 85.15% -0.00% 84.08%
Hits 107980 108138 +158 169
Misses 18827 18858 +31 32

Diff coverage report

Full coverage report

Modules coverage

Coverage did not change

Full coverage reports

Reports

Warnings

  • framework new line coverage rate 84.08% is less than the suggested 90.0%

This comment will be updated on new commits.

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.

4 participants