-
Notifications
You must be signed in to change notification settings - Fork 239
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
Mesh on mesh mapping fix #2390
Mesh on mesh mapping fix #2390
Conversation
@@ -151,7 +134,7 @@ int main (int argc, char* argv[]) | |||
// maps the elevation of mesh nodes according to a ground truth mesh whenever nodes exist within max_dist | |||
if (current_key == "-MESH") | |||
{ | |||
if (argc < 5) | |||
if (argc < 4) |
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.
For another PR; this cli arguments handling could be upgraded to tclap.
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 know. However, this would require more time than I can currently spend on this. It's on the 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.
Looks ok. Didn't check the details of the algorithm change in moveMeshNodes.cpp
{ | ||
|
||
/// Returns the element in which the given node is located when | ||
/// projected onto a mesh, or nullptr if no such element was found. |
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.
Doxygen hasn't finished yet, but the function documentation appears twice in the code, once here, once in the header file; it could be that it will show up twice in the doxygen's function documentation. Needs a quick check, when doxygen is done.
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.
Removed the duplicate comments.
Codecov Report
@@ Coverage Diff @@
## master #2390 +/- ##
==========================================
+ Coverage 32.72% 32.78% +0.06%
==========================================
Files 529 530 +1
Lines 19957 19816 -141
Branches 9449 9302 -147
==========================================
- Hits 6530 6496 -34
+ Misses 10089 9966 -123
- Partials 3338 3354 +16
Continue to review full report at Codecov.
|
The change to the algorithm is really simple: The utility used to assign the elevation of the closest point in the ground truth mesh to all nodes in the mesh to be mapped. Now, it using the actual elevation at the location of each node (that's the functionality introduced in the mesh2raster tool) and only if no point is found, it checks for the closest point within a user-specified radius (which is useful if boundaries are slightly translated due to different precision used in preprocessing tools). |
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.
👍
@rinkk This needs a fix for an unused 'offset' variable in moveMeshNodes.cpp:144 |
OpenGeoSys development has been moved to GitLab. |
Adjusts the mesh on mesh mapping tool to use the functionality introduced in #2367