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

Let users use variables as coordinates to a function in an auxkernel #27081

Merged
merged 4 commits into from
Apr 18, 2024

Conversation

GiudGiud
Copy link
Contributor

@GiudGiud GiudGiud commented Mar 14, 2024

I have a project where we have function data that is hardly convertible to another format (eg we would need to write an object that does what the function reading the csv does)
and an auxiliary variable that is spatial but not x y z (it s a distance to a wall). So it would fit great as a function argument conceptually. Unfortunately, ParsedFunction does not accept auxvariables.

So our two options (aside from this) would be:

  • create a new matprop or auxkernel that includes the function work, and either includes the wall distance computation work or accept an aux variable. Keep this object in an app since it cant go in MOOSE
  • create a new matprop or auxkernel that takes the function and the auxvariable as arguments and does this operation we have here but with dedicated names and only supports the 1 need we have. Also cant go in moose and it s just a less general version of this object

The concerns with having functions depend on variables:

  • no automated dependency resolution that ensures the auxvariables are up to date when they are used in the function as arguments. Some of the functors (PPs) are not the best at this anyway
  • blurs the lines between systems. Functions should be xyzt, AuxVariables kind of an output tool and material properties for having actual variable dependencies in terms that come in the equations
  • loses derivatives. But that s the case with ALL auxkernels so that concern is not new

refs #26784

@youyeonc

@GiudGiud GiudGiud self-assigned this Mar 14, 2024
@GiudGiud GiudGiud marked this pull request as ready for review March 14, 2024 04:20
@moosebuild
Copy link
Contributor

moosebuild commented Mar 14, 2024

Job Documentation on 95a9ed7 wanted to post the following:

View the site here

This comment will be updated on new commits.

@moosebuild
Copy link
Contributor

moosebuild commented Mar 14, 2024

Job Coverage on 95a9ed7 wanted to post the following:

Framework coverage

a489a5 #27081 95a9ed
Total Total +/- New
Rate 85.12% 85.12% +0.00% 92.86%
Hits 102933 102974 +41 39
Misses 18001 18002 +1 3

Diff coverage report

Full coverage report

Modules coverage

Coverage did not change

Full coverage reports

Reports

This comment will be updated on new commits.

@dschwen
Copy link
Member

dschwen commented Mar 14, 2024

That title is confusing. It sounds like you want to let users pass variable names as arguments to parameters of type FunctionName. What you are doing is providing an AuxKernel that evaluates a Function at xyzt coordinates determined through functors. Does not sound like "Abuse", so I vote for fixing that name.

@GiudGiud GiudGiud changed the title Let users use variables as function arguments in auxkernels Let users use variables as coordinates to a function in an auxkernel Mar 14, 2024
@GiudGiud
Copy link
Contributor Author

Fixed the pr title.
Thoughts for the name?

FunctionWithFunctorCoordinatesAux?
FunctorPositionFunctionAux?

@GiudGiud
Copy link
Contributor Author

@dschwen thoughts on picking a new name?

@GiudGiud
Copy link
Contributor Author

Now that you are back @dschwen thoughts for the name?

@dschwen
Copy link
Member

dschwen commented Mar 26, 2024

CoupledVariableCoordinateFunctorAux?

@GiudGiud
Copy link
Contributor Author

It s more of a FunctionAux since the Function is used for the evaluations
and the variables are not coordinates, I use functor for those

so maybe FunctorCoordinatesFunctionAux ?

@GiudGiud GiudGiud requested a review from dschwen March 28, 2024 15:46
@GiudGiud GiudGiud force-pushed the PR_function_abuse branch 2 times, most recently from d7785cf to 63b8fb6 Compare March 29, 2024 00:41
@GiudGiud
Copy link
Contributor Author

Failures are unrelated

@GiudGiud
Copy link
Contributor Author

@lindsayad @dschwen

Copy link
Member

@lindsayad lindsayad left a comment

Choose a reason for hiding this comment

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

I actually very much like this PR

rename to FunctorCoordinatesFunctionAux
- allow for linear finite volume variables
- add note about auxkernel execution and variable updates
@GiudGiud
Copy link
Contributor Author

Test failures unrelated.
thanks for the review!

@GiudGiud GiudGiud merged commit fdb5614 into idaholab:next Apr 18, 2024
45 of 47 checks passed
@GiudGiud GiudGiud deleted the PR_function_abuse branch April 18, 2024 18:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants