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

Move CoupledGradientMaterial into framework and increase capabilities #30006

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

Conversation

tophmatthews
Copy link
Contributor

Ref #15915

@moosebuild
Copy link
Contributor

moosebuild commented Mar 3, 2025

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

View the site here

This comment will be updated on new commits.

@moosebuild
Copy link
Contributor

moosebuild commented Mar 3, 2025

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

Framework coverage

2aabd6 #30006 55b715
Total Total +/- New
Rate 85.26% 85.27% +0.01% 94.74%
Hits 108581 108604 +23 18
Misses 18769 18765 -4 1

Diff coverage report

Full coverage report

Modules coverage

Coverage did not change

Full coverage reports

Reports

This comment will be updated on new commits.

#include "Material.h"

/**
* A material that optinally computes two properties, one corresponding to the value of the coupled
Copy link
Contributor

Choose a reason for hiding this comment

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

I dont think it does two properties.
I think it only compute the gradient one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

crap, those are comments from the original header.

Comment on lines 29 to 34
GenericMaterialProperty<RealVectorValue, is_ad> & _grad_mat_prop;
const GenericMaterialProperty<Real, is_ad> & _scalar_property;
const GenericVariableGradient<is_ad> & _grad_u;
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
GenericMaterialProperty<RealVectorValue, is_ad> & _grad_mat_prop;
const GenericMaterialProperty<Real, is_ad> & _scalar_property;
const GenericVariableGradient<is_ad> & _grad_u;
/// Material property computed, equal to the gradient of the variable
GenericMaterialProperty<RealVectorValue, is_ad> & _grad_mat_prop;
/// A scalar material property that acts as a factor in the computed property
const GenericMaterialProperty<Real, is_ad> & _scalar_property;
/// Gradient of the variable
const GenericVariableGradient<is_ad> & _grad_u;

_scalar_property(getGenericMaterialProperty<Real, is_ad>("scalar_property")),
_grad_u(coupledGenericGradient<is_ad>("u"))
{
}
Copy link
Contributor

Choose a reason for hiding this comment

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

check if u is finite volume, and if so error because grad_u = 0 is not interesting

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hmm...whats the best way to do that?

Copy link
Contributor

Choose a reason for hiding this comment

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

get a pointer to the variable and do:

if (var.isFV())
mooseError

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wait, is that true? This was originally used in fvkernels/mms/grad-reconstruction.mat-rz for a FV variable. If I replace the gradient with all zeros, it diffs.

Copy link
Contributor

Choose a reason for hiding this comment

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

interesting! i didnt know this worked

CoupledGradientMaterialTempl<is_ad>::validParams()
{
InputParameters params = Material::validParams();
params.addClassDescription("Create a gradient material coupled to a variable.");
Copy link
Contributor

Choose a reason for hiding this comment

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

reword

# CoupledValueFunctionMaterial / ADCoupledGradientMaterial

!syntax description /Materials/CoupledGradientMaterial

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 add to the stub. ideally you would mention an example, maybe even how you are using it

params.addParam<MaterialPropertyName>(
"scalar_property",
1.0,
"Scalar material property multiplied by the coupled variable value and gradient.");
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
"Scalar material property multiplied by the coupled variable value and gradient.");
"Scalar material property acting as a factor in the output gradient material property.");

"times the scalar.");
params.deprecateParam("grad_mat_prop", "gradient_material_name", "12/12/25");
params.addParam<MaterialPropertyName>(
"scalar_property",
Copy link
Contributor

Choose a reason for hiding this comment

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

I d deprecate this name and call it scalar_property_factor

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No need to deprecate this one, it is new. I'll change the name.

"scalar_property",
1.0,
"Scalar material property multiplied by the coupled variable value and gradient.");
params.addRequiredCoupledVar("u", "The coupled variable");
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
params.addRequiredCoupledVar("u", "The coupled variable");
params.addRequiredCoupledVar("u", "The coupled variable to take the gradient of");

@tophmatthews tophmatthews force-pushed the coupled_grad_mat_ad_15915 branch from 4559867 to b7b3363 Compare March 6, 2025 17:24
@tophmatthews tophmatthews force-pushed the coupled_grad_mat_ad_15915 branch from b7b3363 to 1c4e8b4 Compare March 6, 2025 17:27
@GiudGiud GiudGiud self-assigned this Mar 6, 2025
@tophmatthews tophmatthews marked this pull request as ready for review March 7, 2025 13:06
@tophmatthews tophmatthews requested a review from lindsayad as a code owner March 7, 2025 13:06
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