-
Notifications
You must be signed in to change notification settings - Fork 170
"I'm done with ptycho" -- George in August 🤦 #695
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
Conversation
Extending functionality to other ptycho classes
Segmented and Other Geometry Detector Ptychography
|
Direct ptychography tutorials for review: direct_ptychography_tutorials.zip |
|
Virtual detector ptychography notebook for review: virtual_detector_ptycho_notebook.zip |
|
Alrighty, this should be ready to review. A lot of this code has been used in the wild this past month by various collaborators, so perhaps they can help us review this (I've added three sets of notebooks above to help with the review):
Thanks! |
|
Apologies for the delay! Having already used the code when it was being developed to good results, I've now additionally reviewed the direct ptycho tutorials. They run perfectly and are mostly really clear! A few minor suggested corrections to direct_ptychography_01.ipynb below:
Regarding the OBF method: I was discussing this with the folks Tokyo, and apparently JEOL has a patent on this and so you might get into some hot water by publishing it... I couldn't quite get a clear "No you definitely shouldn't do it" or "Go for it, it's fine", probably because it might depend on who hears about this at the company and how they react. |
TomaSusi
left a comment
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.
It is a LOT of code to review line-by-line, but having tested the functionality fairly extensively and having had a read through the docstrings (with two comments left), I think it can be approved.
|
I have also been using the |
|
Great, thanks @TomaSusi and @smribet! This is pretty much ready on my end, with the following two caveats:
|
|
Sorry for the delay on my part as well. I've been using the virtual detector ptycho code and that works well. I'm happy to call this ready too, with the exception that I've tested the mpire attempted fix for Windows machines and there still seems to be some issue there. |
|
Hey everyone, sorry for the delay. I am finished with the reviews now: About Parallax_01.ipynb
About Parallax_02.ipynb
About changes from all commits in Github:
which are very consistent with what you explain in your changes log. So it looks good. |
adding interpolation order flag for shifting, default bicubic
|
Thanks @kucukogluberk and @juliedactyl! Re: the mpire windows bug. Turns out this is rather annoying and not simply fixed by In windows the "fork" I don't think this use-case warrants us adding So, I added some logic to check if the system is Windows, set is_windows = platform.system() == "Windows"
if is_windows:
try:
from multiprocess import RawArray
except (ImportError, ModuleNotFoundError) as exc:
raise Exception(
(
'On Windows, num_jobs>1 requires the additional "multiprocess" package. '
"Please install it and try again. Note this is probably inadvisable."
)
) from exc
else:
from multiprocessing import RawArray |
This PR adds a number of new features, namely:
slim_preprocessfunctionality in most (all?) ptycho classespreprocessand allows one to pass in preprocessed arrays directlySSB: (phase-compensated) single-side band reconstructionWDD: Wigner distribution deconvolutionOBF: optimum bright-field STEMWill add tutorial notebooks to check/review below later today.