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

[feature] gridding utils #90

Merged
merged 40 commits into from
Aug 10, 2020
Merged

[feature] gridding utils #90

merged 40 commits into from
Aug 10, 2020

Conversation

pc494
Copy link
Member

@pc494 pc494 commented Jul 28, 2020

#89 but to the correct branch.

As detailed elsewhere #84 we're porting (some) of the gridding functionality from diffsims to orix. I will also bump the matplotlib version in setup.py.

Release Notes:
orix can now produce uniform grids of rotations around a fixed rotations, or within a stated fundamental zone.

Notes:
This will close #85

@hakonanes hakonanes self-requested a review July 28, 2020 11:08
@dnjohnstone
Copy link
Member

Will this address #86 also?

@pc494
Copy link
Member Author

pc494 commented Jul 28, 2020

#86 is going back to diffsims I think, as I'm not doing stereographic projection stuff in orix (but need a few of the utils that I had made - in diffsims- for the ZAP map generator).

@dnjohnstone
Copy link
Member

dnjohnstone commented Jul 28, 2020

#86 is going back to diffsims I think, as I'm not doing stereographic projection stuff in orix (but need a few of the utils that I had made - in diffsims- for the ZAP map generator).

Ok - when this one is done, I think maybe we should remove the stereographic_projection stuff from diffsims, i.e. still get rid of all the gridding stuff from there, since we can use this fundamental zone stuff instead and just leave the issue in orix as "for the future" functionality. Does that sound ok?

@pc494
Copy link
Member Author

pc494 commented Jul 28, 2020

Yep, just wanted to do it carefully so as not to break the zap map code. Normal template matching should be done with fundamental zone grids.

@pc494 pc494 marked this pull request as ready for review July 31, 2020 13:32
@pc494
Copy link
Member Author

pc494 commented Aug 9, 2020

I think I have found the bug, if the samples created post 6495611 are still uniform (according to MTEX) I think they will have a sensible number of n ~ 161 914 / 2 (if you could check that would be must appreciated @hakonanes)

Assuming that has worked I would hope to wrap this (and my review of #92) early next week which puts us in a good place wrt v0.4

@hakonanes
Copy link
Member

Pulling the recent changes and sampling point group 32 with get_sample_fundamental(2.273, Oh) I get 83 693 orientations. The ODF computed with MTEX with default parameters has texture index 1.0084, entropy -0.0046 and max intensity 1.1160. Before 6495611, these four numbers were 161 914 1.0028, -0.0016, 1.0406.

The number of orientations is halfed, while the sampling seems to retain most of its uniformity!

@hakonanes
Copy link
Member

hakonanes commented Aug 9, 2020

Plotting 10 000 random orientations from the samples with MTEX:

Orix (83 693 orientations) EMsoft (72 227) MTEX (54514)
sampling_orix sampling_emsoft sampling_mtex

@hakonanes
Copy link
Member

I would just like to note that I hope we (I) can implement the orientation sampling procedure from EMsoft as well... Most likely as part of a later release, as my hack included above requires multiple optimizations and generalising to all crystal systems.

@pc494
Copy link
Member Author

pc494 commented Aug 9, 2020

Okay, my feelings are:

  • I can tidy this up to something that we can merge, with functionality as is
  • The different number of orientations is due to ambiguous definitions of "resolution"
  • The function uniform_sample_SO3 could easily have a kwarg added in a future release for other methods (eg EMsoft's method)

I propose that I perform the tidy up here and get it merged, and I'll raise a new issue to cover the latter two bullet points.

@hakonanes
Copy link
Member

I propose that I perform the tidy up here and get it merged, and I'll raise a new issue to cover the latter two bullet points.

Sounds good.

@dnjohnstone dnjohnstone added this to the v0.4.0 milestone Aug 10, 2020
@pc494
Copy link
Member Author

pc494 commented Aug 10, 2020

This is now ready to review :)

Copy link
Member

@hakonanes hakonanes left a comment

Choose a reason for hiding this comment

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

Minor docstring comments, looks good to go otherwise.

orix/sampling/sample_generators.py Outdated Show resolved Hide resolved
orix/sampling/sampling_utils.py Show resolved Hide resolved
@dnjohnstone dnjohnstone added enhancement New feature or request and removed status : wip labels Aug 10, 2020
@pc494
Copy link
Member Author

pc494 commented Aug 10, 2020

I think this is good to go now

@pc494 pc494 mentioned this pull request Aug 10, 2020
@hakonanes hakonanes merged commit 36423e6 into pyxem:master Aug 10, 2020
@hakonanes
Copy link
Member

hakonanes commented Aug 10, 2020

I took (for the first time) the liberty of merging... If you guys @pc494 @dnjohnstone want that task to continue to be your responsibility alone, I understand that.

@pc494
Copy link
Member Author

pc494 commented Aug 10, 2020

Nope that's great, it means I can cross "figure out how to get @hakonanes made an admin on orix" off my list!

@dnjohnstone
Copy link
Member

I took (for the first time) the liberty of merging... If you guys @pc494 @dnjohnstone want that task to continue to be your responsibility alone, I understand that.

Absolutely great @hakonanes ! I think you've got more claim to that than me these days!

@hakonanes
Copy link
Member

Glad to hear it, thanks a lot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Uniformity of grids in SO3
3 participants