Document default Delaunay Triangulation algorithm as Shewchuk #6438
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Current default should be the Shewchuk [1996] algorithm, and not Watson [1982]. Modified documentation in contour.rst, triangulate.rst and gmt.conf.rst to be clear on this. Also mentioned using `gmt get GMT_TRIANGULATE` as the way to know which one is being used.
weiji14
commented
Mar 12, 2022
Comment on lines
17
to
+23
| In makegmt.macro there are two parameters that can be defined: | ||
| TRIANGLE_D and TRIANGLE_O. By default they are commented out. | ||
| TRIANGLE_D and TRIANGLE_O. By default these lines are commented out. | ||
| To use Shewchuk's [1996] routine, remove the # from the | ||
| beginning of the lines and then 'make clean' and 'make install'. | ||
| To revert to the default GMT usage of Watson's [1982] algorithm, | ||
| To revert to using Watson's [1982] algorithm, | ||
| simply place a # in the first column at these two lines. | ||
| Then, say 'make clean' and 'make install' again. | ||
| Then, run 'make clean' and 'make install' again. |
There was a problem hiding this comment.
I need a bit of help rewording these lines. I don't see makegmt.macro anywhere (the git blame on this file dates back to Dec 2000!!). I do see a TRIANGLE_D here:
Lines 6580 to 6585 in 434ed18
but no mention of TRIANGLE_O.
There was a problem hiding this comment.
My best guess is that these macros are in reference to the pre-CMake days. I think that this paragraph and the preceding to sentences should instead make these points:
- Use
gmt set GMT_TRIANGULATEto control which algorithm is used on the fly. - Use
set (LICENSE_RESTRICTED GPL)incmake/ConfigUserAdvanced.cmaketo disable Shewchuk's routine entirely.
weiji14
added a commit
to GenericMappingTools/pygmt
that referenced
this pull request
Mar 13, 2022
weiji14
added a commit
to GenericMappingTools/pygmt
that referenced
this pull request
Mar 14, 2022
Wrapping the triangulate function which does "Delaunay triangulation or Voronoi partitioning and gridding of Cartesian data". Original GMT documentation can be found at https://docs.generic-mapping-tools.org/6.3/triangulate.html. Aliased outgrid (G), spacing (I), projection (J), region (R), verbose (V), registration (r). * Refactor triangulate to use virtualfile_from_data * Refactor triangulate implementation to use pygmt.io.load_dataarray * Alias binary(b), nodata(d), find(e), coltypes(f), header(h), incols(i) * Rename the parameter 'table' to 'data' As per #1479. * Refactor test_triangulate to use Table_5_11_mean.xyz instead of tut_ship * Refactor test_triangulate_with_outgrid to use xr.testing.assert_allclose * Refactor test_triangulate_input_xyz to use pd.testing.assert_frame_equal * Implement regular_grid and delaunay_triples staticmethod for triangulate * Let list inputs to spacing (I) and incols (i) work Use I="sequence" and i="sequence_comma". * Ensure triangulate.delaunay_triples output_type is valid Must be either one of numpy, pandas or file * Autocorrect output_type to 'file' if outfile parameter is set * Allow only str or None inputs to outgrid parameter Xref #1807 * Use gmt get GMT_TRIANGULATE to check whether Watson or Shewchuk is used * State that Shewchuk is the default triangulation algorithm As per GenericMappingTools/gmt#6438 * Actually document the output_type parameter for delaunay_triples Co-authored-by: Will Schlitzer <schlitzer90@gmail.com> Co-authored-by: Meghan Jones <meghanj@alum.mit.edu> Co-authored-by: Dongdong Tian <seisman.info@gmail.com>
|
Remind me if this is ready to be approved - it has been languishing for some time. |
|
Might need some more changes to the wording. Feel free to work directly on this branch, I'm a bit short on time right now. |
|
OK, but not sure how to check out your branch... |
|
Here are the instructions for checking out a PR - https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally. That said, I am not sure whether you'd be able to push to Wei Ji's branch (I haven't ever tried to push to a fork from a local checkout). This is an improvement, so IMO you could merge this as a part 1 and make any additional edits as a separate PR if it's easier. |
|
Thanks, that is easier, now merged. |
sixy6e
pushed a commit
to sixy6e/pygmt
that referenced
this pull request
Dec 21, 2022
Wrapping the triangulate function which does "Delaunay triangulation or Voronoi partitioning and gridding of Cartesian data". Original GMT documentation can be found at https://docs.generic-mapping-tools.org/6.3/triangulate.html. Aliased outgrid (G), spacing (I), projection (J), region (R), verbose (V), registration (r). * Refactor triangulate to use virtualfile_from_data * Refactor triangulate implementation to use pygmt.io.load_dataarray * Alias binary(b), nodata(d), find(e), coltypes(f), header(h), incols(i) * Rename the parameter 'table' to 'data' As per GenericMappingTools#1479. * Refactor test_triangulate to use Table_5_11_mean.xyz instead of tut_ship * Refactor test_triangulate_with_outgrid to use xr.testing.assert_allclose * Refactor test_triangulate_input_xyz to use pd.testing.assert_frame_equal * Implement regular_grid and delaunay_triples staticmethod for triangulate * Let list inputs to spacing (I) and incols (i) work Use I="sequence" and i="sequence_comma". * Ensure triangulate.delaunay_triples output_type is valid Must be either one of numpy, pandas or file * Autocorrect output_type to 'file' if outfile parameter is set * Allow only str or None inputs to outgrid parameter Xref GenericMappingTools#1807 * Use gmt get GMT_TRIANGULATE to check whether Watson or Shewchuk is used * State that Shewchuk is the default triangulation algorithm As per GenericMappingTools/gmt#6438 * Actually document the output_type parameter for delaunay_triples Co-authored-by: Will Schlitzer <schlitzer90@gmail.com> Co-authored-by: Meghan Jones <meghanj@alum.mit.edu> Co-authored-by: Dongdong Tian <seisman.info@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of proposed changes
Current default should be the Shewchuk [1996] algorithm, and not Watson [1982]. Modified documentation in contour.rst, triangulate.rst, gmt.conf.rst and TRIANGLE.HOWTO to be clear on this. Also mentioned using
gmt get GMT_TRIANGULATEas the way to know which one is being used.Fixes #6436
Reminders