-
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
Enable element activation capability #16008
Conversation
Job Documentation on 335d102 wanted to post the following: View the site here This comment will be updated on new commits. |
585552b
to
06b3a05
Compare
@fdkong maybe you are the best person to review this? @jiangwen84 please add here any comment you may have. Thanks! |
@fdkong any comments on this PR? Just in case you may have forgotten this on your to-do list :) |
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 looks impressive. Just a few comments
/// define the distance of the element to the point on the path, | ||
/// below which the element will be activated | ||
const Real _activate_distance; | ||
/// varaible to decide wether an element whould be activated |
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.
Typo: varaible --> variable
_expand_boundary_name[0]; | ||
displaced_problem->mesh().getMesh().get_boundary_info().nodeset_name(_disp_boundary_ids[0]) = | ||
_expand_boundary_name[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.
You have so many 0
here. Do you actually need "std::vector"
? If there is one component only, you do not need to use "std::vector"
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.
I am not sure what to do here. I meant to add the expanded boundary if that does not already exist. But the function with this capability is only this one:
std::vector<BoundaryID> getBoundaryIDs(const std::vector<BoundaryName> & boundary_name, bool generate_unknown = false)
That is the major reason I am using std::vector
for these. Any suggestions?
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.
Then just do assert
mooseAssert(!boundary_name.empty())
mooseAssert(!_boundary_ids.empty())
mooseAssert(!_disp_boundary_ids.empty())
ece72b7
to
5255be7
Compare
|
||
This user object mimics the process of activating (adding) an element by moving the element from an "inactive" subdomain to the "active" subdomain. There are two metrics for activating an element, i.e., by +function path+ and by +variable value+. | ||
|
||
- Activation by +function path+ uses an user provided path function $f(x(t),y(t),z(t))$ and activates an element if this element is close to the location of the path, i.e., $f(x(t_0), ,y(t_0),z(t_0))$ at time $t_0$. The path function is set by `function_x`, `function_y`, and `function_z` in the input block. |
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 is no f, this should be the "point $(x(t), y(t), z(t))$
with components defined by the functions specified by the parameters function_x
, function_y
, and function_z
in the input."
framework/doc/content/source/userobject/ActivateElementUserObject.md
Outdated
Show resolved
Hide resolved
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 looks great for me! Thanks.
@dewenyushu You need to address all comments from @dschwen before you can merge it. |
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.
Good for me. But you need to address @dschwen 's comments.
5255be7
to
2db2a80
Compare
2db2a80
to
34be2de
Compare
Job App tests (Non Conda) on 34be2de : invalidated by @dschwen |
Job App tests (Non Conda) on 34be2de : invalidated by @fdkong Is this real? test:materials/fv_damper.seismic FAILED (CSVDIFF) |
@fdkong I am not sure why that is happening. That's odd. |
@cbolisetti thanks for looking into the failed test in Mastodon. Any luck with it? |
Hi Dewen, not yet. I’ll have to take a look at it tonight or over the
weekend. Sorry for the delay..it’s a bit of a busy week.
On Wed, Nov 18, 2020 at 1:28 PM dewenyushu ***@***.***> wrote:
Job App tests (Non Conda) <https://civet.inl.gov/job/621677/> on 34be2de
<34be2de>
: invalidated by @fdkong <https://github.com/fdkong>
Is this real? test:materials/fv_damper.seismic FAILED (CSVDIFF)
@fdkong <https://github.com/fdkong> I am not sure why that is happening.
That's odd.
@cbolisetti <https://github.com/cbolisetti> thanks for looking into the
failed test in Mastodon. Any luck with it?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#16008 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACQ6BU6DWKJKJSBPJQNZCHTSQQVAJANCNFSM4TAEJR3A>
.
--
Sent from Mobile
|
Oh no worries. Just let me know if there would be anything I can help with the fix. |
34be2de
to
2aaffd0
Compare
{ | ||
InputParameters params = ActivateElementsUserObjectBase::validParams(); | ||
|
||
params.addParam<FunctionName>("function_x", "The x component heating spot travel path"); |
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.
params.addParam<FunctionName>("function_x", "The x component heating spot travel path"); | |
params.addParam<FunctionName>("function_x", "The x component of the heating spot travel path"); |
{ | ||
// activate center (assume position of the activate center is only time dependent) | ||
const static Point dummy; | ||
Real x_t = _function_x->value(_t, dummy); |
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.
Real x_t = _function_x->value(_t, dummy); | |
Real x_t = _function_x ? _function_x->value(_t, dummy) : 0.0; |
Real avg_val = 0.0; | ||
|
||
for (unsigned int qp = 0; qp < _qrule->n_points(); ++qp) | ||
avg_val += (*_coupled_var)[qp]; |
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.
Looks like _coupled_var
could be nullptr
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.
Good point! Can we make _coupled_var
a required parameter? Otherwise I may just throw paramError
when it is nullptr
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.
Absolutely. I just realized that both this and the functions were only pointers because you had them in the same class before. Make it required and use reference.
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.
Thanks @dschwen ! Just to make sure, by this you mean something like:
const VariableValue _coupled_var;
in the header,
params.addRequiredCoupledVar("coupled_var", " ");
in the ActivateElementsCoupled::validParams()
, and
_coupled_var(coupledValue("coupled_var"))
in the constructor?
The function paths maybe 1, 2 or 3D for different cases so it maybe better to not make them required, I guess?
params.addRequiredParam<subdomain_id_type>("active_subdomain_id", "The active subdomain ID."); | ||
params.addParam<subdomain_id_type>( | ||
"inactive_subdomain_id", | ||
-1, |
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.
subdomain_id_type
is an unsigned integer type. Better use libMesh::invalid_uint
here!
2aaffd0
to
d3c7a41
Compare
* This is needed when elements/boundary nodes are added to a specific subdomain | ||
* at an intermediate step | ||
*/ | ||
void initElementStatefulProps(ConstElemRange & elem_range); |
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.
void initElementStatefulProps(ConstElemRange & elem_range); | |
void initElementStatefulProps(const ConstElemRange & elem_range); |
params.addParam<FunctionName>("function_x", "The x component of the heating spot travel path"); | ||
params.addParam<FunctionName>("function_y", "The y component of the heating spot travel path"); | ||
params.addParam<FunctionName>("function_z", "The z component of the heating spot travel path"); |
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.
Dewen, I'm sorry, I should probably have suggested to just add a default value of 0
for the function. Then you don't need to use pointer and pointer checking. This could be done later though, as it won't change the interface.
params.addParam<FunctionName>("function_x", "The x component of the heating spot travel path"); | |
params.addParam<FunctionName>("function_y", "The y component of the heating spot travel path"); | |
params.addParam<FunctionName>("function_z", "The z component of the heating spot travel path"); | |
params.addParam<FunctionName>("function_x", "0", "The x component of the heating spot travel path"); | |
params.addParam<FunctionName>("function_y", "0", "The y component of the heating spot travel path"); | |
params.addParam<FunctionName>("function_z", "0", "The z component of the heating spot travel path"); |
const MeshBase::const_element_iterator elems_begin = | ||
MeshBase::const_element_iterator(elempp, elemend, Predicates::NotNull<Elem * const *>()); | ||
|
||
const MeshBase::const_element_iterator elems_end = |
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.
I'd use auto
in these cases
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 looks great. If you address the minor comments it should be ready to go.
d3c7a41
to
ef99a5b
Compare
Job Precheck on ef99a5b 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:
|
…vateElementsByPath classes Add methods in FEProblemBase in support of element activation Add tests Refs. issue idaholab#15795
ef99a5b
to
335d102
Compare
Add
ActivateElementUserObject
class to enable element activation by moving elements from the 'inactive' subdomain to the 'active' subdomain.Add methods in
FEProblemBase
in support of element activationAdd tests to make sure:
elements can be activated by path or by variable value
initial conditions are properly projected to the activated elements
material properties are initialized properly for the activated elements
Refs. issue #15795