-
Notifications
You must be signed in to change notification settings - Fork 144
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
moire lattice in Crystal
#514
Conversation
Testing notebook for Moire: |
This works with the example notebook and should be ready for review |
Adding requested docstrings
The fix @alex-rakowski made should fix the one failing check on the docs, so it should be okay to merge with that one failing. Also if we push new commits (or even just an empty one) the run will trigger again and we can know for sure. In the dev meeting I believe @gvarnavi was volunteered to review this |
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 great overall, but I think the function should be split in two functions to be more consistent w/ the generate
/plot
scheme of the rest of the diffraction module.
This requires the generate
function to return some information on the thresholded and rotated parent classes. Here, I simply return them as different PointList
s
Usage now looks like this:
bragg_parent_0, bragg_parent_1, bragg_moire = generate_moire_diffraction_pattern(
bragg_peaks_WSe2,
bragg_peaks_Te,
k_max = 0.65 ,
exx_1 = 0.0,
eyy_1 = 0.05,
exy_1 = 0.0,
phi_1 = np.deg2rad(2),
)
fig, ax = plot_moire_diffraction_pattern(
bragg_parent_0,
bragg_parent_1,
bragg_moire,
k_max = 0.65,
plot_subpixel = True,
labels = ['WSe$_2$', 'Te'],
returnfig=True,
)
Co-authored-by: Georgios Varnavides <[email protected]>
@gvarnavi do you approve this in its current state? I can't review it on GitHub because I am the author of the PR |
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.
Lgtm! Although I believe @cophus wanted to have one final look?
This should be a cleaned up and non-conflicting version of #511
This version is formatted and does not contain any changes to the CrystalPhase workflow.
Needs testing