Skip to content
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

Creating spherical meshes for get_grid_beam_directions #129

Closed
pc494 opened this issue Sep 28, 2020 · 7 comments
Closed

Creating spherical meshes for get_grid_beam_directions #129

pc494 opened this issue Sep 28, 2020 · 7 comments
Assignees
Labels
enhancement New feature, request, or improvement
Milestone

Comments

@pc494
Copy link
Member

pc494 commented Sep 28, 2020

Is your feature request related to a problem? Please describe.
get_grid_beam_directions currently picks a mesh in a rather naive way. As described in [1] several "better" options exist. A fairly extensive discussion can be found in #128. Thanks are due to @din14970 for bringing this to my attention.

Describe the solution you'd like
This remains slightly up for discussion. From a programming point of view, I think it's fairly obvious that we want
get_grid_beam_directions(crystal_system,resolution,equal="area")
to become get_grid_beam_directions(crystal_system,resolution,mesh="mesh_type")

The questions that are then open are
(a) how many mesh types should we provide?
(b) how should be describe "resolution" for these mesh types

My inclination on (a) is that options 1-3 from the linked article would be enough, and adding an extra package for what at first glance seems a fairly marginal benefit feels wrong to me.

On (b) I think the resolution should probably the worst nearest neighbour distance on the grid, although this is a conservative number. Regardless, we should agree what it is, write it in the docstrings, and use it for all gridding types.

Also, a more formal citation to replace [1] would be good, but that's a bonus.

=====

@din14970 are you happy to be assigned this?

[1] https://medium.com/game-dev-daily/four-ways-to-create-a-mesh-for-a-sphere-d7956b825db4

@pc494 pc494 added the enhancement New feature, request, or improvement label Sep 28, 2020
@pc494 pc494 added this to the v0.4.0 milestone Sep 28, 2020
@din14970
Copy link
Contributor

@pc494 yes I can try to handle it. Might need some input on proper unit testing along the way though.

In regards to the remaining questions:

(a) agree 1-3 are a must, 1 is already there as equal=angle and 2-3 are just my methods that should be slightly modified. An icosahedral mesh would be nice but since it is generated iteratively it doesn't play nice with the resolution argument. One might not need the entire meshzoo package, just copy the relevant code bits for meshing a sphere. They are licensed under GPL-3, this should mean the code can be copied because diffsims is also GPL-3, right? I will experiment with it and see whether it's worth it, then we can discuss in the PR.

(b) yes, I would also opt for largest nearest neighbor distance in degrees. In the UV sphere this is on the equator, in the cube based meshes this is the angle from [001] to its nearest neighbor in the direction of the [111] corner.

Will try to find a proper reference for sphere meshing

@pc494
Copy link
Member Author

pc494 commented Sep 28, 2020

Sounds good to me 👍

@pc494
Copy link
Member Author

pc494 commented Oct 4, 2020

Having thought some more on this, I now feel even more strongly that only providing "spherical", "normalized-cube" and "spherified-cube" of the mesh methods is the best option at this point.

@din14970
Copy link
Contributor

din14970 commented Oct 4, 2020

Shame, I just got it to work for the icosahedral grid. Here is a grid with maximum angular difference between nearest neighbors of 2 degrees:
image
Looks quite nice and fair, although you obviously still see the underlying icosahedron. Main downside to this method is that it is generated fairly slowly because it is generated iteratively. Reason is I don't know a priori what the maximum angle between nearest neighbors will be.

Here are the grids for the other meshes:
UV sphere
image
Normalized cube
image
Spherified cube
image

There's two holes at 2 {111} poles for some reason so I have to find what the issue there is. For a cubic crystal, the spherified cubic meshing is obviously the best. I've found that there are also two ways of spherifying the cubic grid: either equalizing the angles between grid points from 001 to 011 (shown above) or equalizing the angles between 001 and 111 (what I had before and shown below).

image

The difference is very minor but I'm inclined to also allow for the two options through an additional kw argument.

@pc494
Copy link
Member Author

pc494 commented Oct 4, 2020

It would be a shame to see good work go to waste, feel free to include all 4 (+1) methods in a PR and I'll have a think

@dnjohnstone
Copy link
Member

This is really good - thanks everyone involved! I'd vote for the more the merrier right now I think...

@pc494
Copy link
Member Author

pc494 commented Oct 8, 2020

closed by #130

@pc494 pc494 closed this as completed Oct 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature, request, or improvement
Projects
None yet
Development

No branches or pull requests

3 participants