[Core] Small repair of normalize NORMAL in coordinate_transformation_utilities #5287
Conversation
bodhinandach
added
Kratos Core
FastPR
| Norm = sqrt(Norm); | ||
| for(typename TVectorType::iterator iComponent = rThis.begin(); iComponent < rThis.end(); ++iComponent) | ||
| *iComponent /= Norm; | ||
| Norm += (*iComponent)*(*iComponent); |
There was a problem hiding this comment.
Please reformat this as it used to be.
There was a problem hiding this comment.
do you mean the tab? I dont change any other than tabbing it for clarity of the loop. But sure, I can return it back.
| *iComponent /= Norm; | ||
| Norm += (*iComponent)*(*iComponent); | ||
| Norm = std::sqrt(Norm); | ||
| if (Norm > std::numeric_limits<double>::epsilon()){ |
There was a problem hiding this comment.
What happens if the Norm is 0? IMO you should at least throw a warning.
There was a problem hiding this comment.
So, if the norm is zero, it means that all the normal vector component is zero. You should not divide by zero.
There was a problem hiding this comment.
This happens to me only for moving slip surface. Which is completely normal, and doesn't require any warning.
There was a problem hiding this comment.
@bodhinandach could you please clarify when do you encounter this situation (zero normal) and what would you expect to happen in that case? I would think that a normal with (close to) zero norm is quite useless to determine the correct orientation of the slip condition so, if anything, I would either ignore the rotation (and not rotate the affected nodes) or throw an error.
There was a problem hiding this comment.
@jcotela So, in my case, I map normal vector from some type of "boundary" particles to the background node, for some slip boundary condition imposition, by using shape functions. These boundary particles can move through different background meshes.
If, suppose that at one point the particle is exactly or very closely located to one node, and therefore, the shape functions to the other nodes go to zero, zero normals can be found in those nodes. But as it is a temporary variable, and the information will be deleted in the next time step, it is normal for me to encounter this issue, and therefore such warning is not needed.
There was a problem hiding this comment.
Okay, then I will derive the
Normalize()in my utilities, and add the warning in the core?
I'll derive it in your class and keep the core one as it is.
There was a problem hiding this comment.
Yes, to avoid the if overhead. But let's wait for @jcotela 's opinion.
There was a problem hiding this comment.
I returned it back to what it is, if you want to add the error, I can also add it. Waiting for your decision.
There was a problem hiding this comment.
I agree that the accepting a zero normal as valid input should be specific of the derived implementation (since it would not work as expected in the general case). I think it would still be good to have some kind of check, even if it is just in debug, for the base class, but I would not block this PR for this.
There was a problem hiding this comment.
Maybe we can add the check only in debug mode. @bodhinandach if you want to do it, feel free to proceed. If not, we can do it in another PR.
|
@rubenzorrilla As discussed with @jcotela, since the function |
@rubenzorrilla @jcotela I make a small check to avoid division by zero when normalizing NORMAL.