Skip to content

Conversation

@pc494
Copy link
Member

@pc494 pc494 commented Jan 23, 2020


name: Rotation lists from streographic gridding
about: As is industry standard we will provide an implementation of rotation list generation that use streographic triangles. See #52

  • ready for review and merge?

Release Notes

major
improvement / bugfix
Summary: Overhaul of streographic functionality

What does this PR do? Please describe and/or link to an open issue.
At a user level this provides a rotation_list that is based on the streographic list, at an internal level this is done by first getting beam directions, and then applying rotations about them, using the 'szxz' trick we tried for get_grid_around_beam_direction, with adjustment illustrated by #53. Theoretically it abuses spherical polar coordinates, as most reduced regions are defined by lines of "longtitude" and the equator, which puts hard limits on theta and phi. The exception is the cubic case.

Work in progress?
If you know there is more to do, make a checklist here:

  • Needs better tests
  • I need to decide if equal area is worth doing
  • I need to fix the cubic case, which currently samples slightly too wide a region
  • I need to write the theory out somewhere
  • Delete old code that did this
  • Make StructureLibrary compliant with all these changes

** Script for checking IPZ figures **
Making use of the headline formulae from https://en.wikipedia.org/wiki/Stereographic_projection#Visualization_of_lines_and_planes

from diffsims.utils.gridding_utils import get_beam_directions
import numpy as np
from matplotlib import pyplot as plt

xyz = get_beam_directions('cubic',5,equal='area')
x = xyz[:,0]
y = xyz[:,1]
z = xyz[:,2]

xs = np.divide(x,1-z)
ys = np.divide(y,1-z)

plt.scatter(xs,ys)

@pc494 pc494 added the enhancement New feature, request, or improvement label Jan 23, 2020
@pc494 pc494 added this to the v0.2.0 milestone Jan 23, 2020
@pc494
Copy link
Member Author

pc494 commented Jan 25, 2020

I'm planning completing the other unticked boxes in a separate PR, which means this is ready for review.

@pc494 pc494 requested a review from dnjohnstone January 25, 2020 16:36
@dnjohnstone
Copy link
Member

I'm planning completing the other unticked boxes in a separate PR, which means this is ready for review.

Could you have a look at the broken tests to start, please? Meanwhile I'll have a look at the rest of it

Copy link
Member

@dnjohnstone dnjohnstone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @pc494 this looks pretty good - but like you got bored around docstring time.

I think a few more comments in the code might be helpful too, but not essential.

This does need a pepstorm - but because that might affect other parts of the code and make review tricky let's leave that for a separate pepstorm only PR.

Nearest neighbour rotations are seperated by a distance of 'resolution'
equal : 'angle' or 'area'
See docstrings for diffsims.utils.gridding_utils.get_beam_directions
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think a brief description here and a "See Also" would be a better solution here.

@dnjohnstone dnjohnstone merged commit 0d18c27 into pyxem:master Jan 27, 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

Successfully merging this pull request may close these issues.

2 participants