-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Linear kepsilon #29847
base: next
Are you sure you want to change the base?
Linear kepsilon #29847
Conversation
226a995
to
81a7048
Compare
… taken into account for linear systems.
… taken into account for linear systems.
8c858aa
to
b590f08
Compare
Job Precheck, step Clang format on b590f08 wanted to post the following: Your code requires style changes. A patch was auto generated and copied here
Alternatively, with your repository up to date and in the top level of your repository:
|
b590f08
to
57a1783
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need some communication about why we are changing the framework objects. Seems to be to allow BCs on aux variables?
@@ -49,7 +49,7 @@ LinearFVBoundaryCondition::LinearFVBoundaryCondition(const InputParameters & par | |||
MooseVariableInterface(this, | |||
false, | |||
"variable", | |||
Moose::VarKindType::VAR_SOLVER, | |||
Moose::VarKindType::VAR_ANY, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why this change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is allowing BCs on aux variables. Peter will do a separate PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, this is in this PR:
#29532
try | ||
{ | ||
computeSystems(EXEC_NONLINEAR); | ||
} | ||
catch (MooseException & e) | ||
{ | ||
_console << "\nA MooseException was raised during Auxiliary variable computation.\n" | ||
<< "The next solve will fail, the timestep will be reduced, and we will try again.\n" | ||
<< std::endl; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we definitely want try-catch
Job Documentation, step Docs: sync website on 57a1783 wanted to post the following: View the site here This comment will be updated on new commits. |
_rhs_non_time_tag(-1), | ||
// We add this vector tag so that objects acting on the aux system inheriting | ||
// from the tagging interface can still be used without any nonlinear systems | ||
_rhs_non_time_tag(_fe_problem.addVectorTag("NONTIME")), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there's a cost to this I think ? Like in memory. Griffin people wont be happy
@@ -91,6 +94,10 @@ LinearSystem::LinearSystem(FEProblemBase & fe_problem, const std::string & name) | |||
// We create a tag for the right hand side, the vector is already in the libmesh system | |||
_rhs_tag = _fe_problem.addVectorTag("RHS"); | |||
|
|||
// We add other wector tags so that objects acting on the aux system inheriting | |||
// from the tagging interface can still be used without any nonlinear systems | |||
_rhs_non_time_tag = _fe_problem.addVectorTag("NONTIME"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are we doing this operation twice? it s right above
for (THREAD_ID tid = 0; tid < libMesh::n_threads(); tid++) | ||
{ | ||
std::vector<LinearFVElementalKernel *> fv_elemental_kernels; | ||
_fe_problem.theWarehouse() | ||
.query() | ||
.template condition<AttribSystem>("LinearFVElementalKernel") | ||
.template condition<AttribThread>(tid) | ||
.queryInto(fv_elemental_kernels); | ||
|
||
for (auto * fv_kernel : fv_elemental_kernels) | ||
fv_kernel->initialSetup(); | ||
|
||
std::vector<LinearFVFluxKernel *> fv_flux_kernels; | ||
_fe_problem.theWarehouse() | ||
.query() | ||
.template condition<AttribSystem>("LinearFVFluxKernel") | ||
.template condition<AttribThread>(tid) | ||
.queryInto(fv_flux_kernels); | ||
|
||
for (auto * fv_kernel : fv_flux_kernels) | ||
fv_kernel->initialSetup(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not use LinearFVKernel for the query instead?
initialSetup exists for this class, since it comes from the "LinearSystemContributionObject " base
@@ -0,0 +1,113 @@ | |||
# LinearFVTKEDSourceSink | |||
|
|||
This kernel implements a version of [`INSFVTKEDSourceSink`](INSFVTKEDSourceSink.md) for a linear kernel. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This kernel implements a version of [`INSFVTKEDSourceSink`](INSFVTKEDSourceSink.md) for a linear kernel. | |
This kernel implements a version of [`INSFVTKEDSourceSink`](INSFVTKEDSourceSink.md) for the linear finite volume discretization of the $k-\epsilon$ turbulence equations. |
same for all other doco files
# LinearFVTKEDSourceSink | ||
|
||
This kernel implements a version of [`INSFVTKEDSourceSink`](INSFVTKEDSourceSink.md) for a linear kernel. | ||
The documentation is repeated down here for completeness. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use a file include instead? Create a new folder above source/
to host incomplete doco files that get included.
This will reduce maintenance cost
// _console << "position: " << _current_elem_info->elem()->vertex_average() << std::endl; | ||
// _console << "y_plus: " << y_plus << std::endl; | ||
// _console << "TKE: " << TKE << std::endl; | ||
// _console << "distance_vec[i]: " << distance_vec[i] << std::endl; | ||
// _console << "destruction: " << destruction << std::endl; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// _console << "position: " << _current_elem_info->elem()->vertex_average() << std::endl; | |
// _console << "y_plus: " << y_plus << std::endl; | |
// _console << "TKE: " << TKE << std::endl; | |
// _console << "distance_vec[i]: " << distance_vec[i] << std::endl; | |
// _console << "destruction: " << destruction << std::endl; |
// Do nothing | ||
// return 0.0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// Do nothing | |
// return 0.0; |
params.addParam<MooseEnum>("wall_treatment", | ||
wall_treatment, | ||
"The method used for computing the wall functions " | ||
"'eq_newton', 'eq_incremental', 'eq_linearized', 'neq'"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it seems only one method is implemented below
are the other s just the same for this kernel?
// Assembly variables | ||
Real destruction = 0.0; | ||
Real tot_weight = 0.0; | ||
std::vector<Real> y_plus_vec, velocity_grad_norm_vec; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you should reserve a size for these two vectors for performance purposes rather than allocating dynamically later on
|
||
// If we are on an internal face, we populate the four entries in the system matrix | ||
// which touch the face | ||
if (_current_face_type == FaceInfo::VarFaceNeighbors::BOTH) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's discuss this tomorrow. this is a ton code duplication for something that block restriction should achieve naturally?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if you dont think so, then let's create "addMatrixContrib/ETCInternal in the advection class.
then in the base class (regular advection) you do: addMatrixContrib() { addMatrixContribInternal() }
and in the derived class, you do
if (on boundary)
code code
else
addMatrixContribInternal();
Closes #29846