-
Notifications
You must be signed in to change notification settings - Fork 248
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
[Core] Add distribute variable method to variable utils #7146
[Core] Add distribute variable method to variable utils #7146
Conversation
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.
Some implementation comments. Other than this, I'm fine with the changes.
Co-authored-by: Rubén Zorrilla <rubenzorrillamartinez@hotmail.com>
Co-authored-by: Rubén Zorrilla <rubenzorrillamartinez@hotmail.com>
Co-authored-by: Rubén Zorrilla <rubenzorrillamartinez@hotmail.com>
Co-authored-by: Rubén Zorrilla <rubenzorrillamartinez@hotmail.com>
…ttps://github.com/KratosMultiphysics/Kratos into core/variable_utils/add_distribute_variable_method
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.
Nothing else from my side. Thanks @sunethwarna !!!
guys please don't open PRs on Sat and merge on Sunday, esp for additions to the core note to self: add to code of conduct |
const std::function<double(const Node<3>&)>& r_weight_method = | ||
(IsInverseWeightProvided) ? | ||
static_cast<std::function<double(const Node<3>&)>>([rWeightVariable](const Node<3>& rNode) -> double {return 1.0 / rNode.GetValue(rWeightVariable);}) : | ||
static_cast<std::function<double(const Node<3>&)>>([rWeightVariable](const Node<3>& rNode) -> double {return rNode.GetValue(rWeightVariable);}); |
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 think using std function has some overhead, use with caution in performance critical parts
- There is no check for
rNode.GetValue(rWeightVariable)
. If it is 0 then it will crash. This happens if this var was not allocate
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 added the check in full debug only. Should I change it to release as well? I didnt do it considering the performance impact. (
Kratos/kratos/utilities/variable_utils.cpp
Lines 532 to 535 in 323771c
KRATOS_DEBUG_ERROR_IF(!r_node.Has(rWeightVariable)) << "Non-historical nodal " << rWeightVariable.Name() << " at " << r_node << " is not initialized in " << rModelPart.Name() << ". Please initialize it first."; - As for the zeros, I can add a check in full debug again. Would it be sufficient?
@philbucher sorry, I wasn't thinking like that. I will keep this in mind in future :/ |
This adds a method to variable_utils to distribute variables(double, and array_3d) in elemental or condition data value container to nodal non-historical data value container with a specified weight. Tests are also added.