Skip to content

Conversation

@gvarnavi
Copy link
Member

This PR adds a number of new features, namely:

  • virtual-detector iterative ptychography
  • slim_preprocess functionality in most (all?) ptycho classes
    • this circumvents preprocess and allows one to pass in preprocessed arrays directly
  • parallax reorganization
    • allows one to input arbitrary aberrations during initialization
    • fit aberrations recursively by radial order
    • better memory-management during upsampling
  • adds (slightly expensive) 3D Fourier-rotation in ptycho-tomo
  • adds multiprocessed direct ptychography classes
    • SSB: (phase-compensated) single-side band reconstruction
      • including recursive aberration-fitting
    • WDD: Wigner distribution deconvolution
    • OBF: optimum bright-field STEM

Will add tutorial notebooks to check/review below later today.

smribet and others added 30 commits August 30, 2024 09:24
Extending functionality to other ptycho classes
Segmented and Other Geometry Detector Ptychography
@gvarnavi
Copy link
Member Author

Direct ptychography tutorials for review: direct_ptychography_tutorials.zip

@gvarnavi
Copy link
Member Author

Virtual detector ptychography notebook for review: virtual_detector_ptycho_notebook.zip

@gvarnavi
Copy link
Member Author

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):

  • @TomaSusi -- can you have a look at the direct ptycho classes, please?
  • @kucukogluberk -- can you have a look at the parallax updates, please?
  • @juliedactyl -- can you have a look at the virtual-detector ptycho updates, please?

Thanks!

@TomaSusi
Copy link

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:

  • Second-to-last cell typo "accuralely"
  • Last cell: it would be clearer to move "reconstructured phase" into a suptitle and name the panels a bit more descriptively. It's also not entirely clear to me why there is a difference between SSB and py4DSTEM SSB?

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.

Copy link

@TomaSusi TomaSusi left a 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.

@smribet
Copy link
Collaborator

smribet commented Jan 15, 2025

I have also been using the phase_contrast branch a lot, and it looks good! Happy to approve and merge once you make the final minor tweaks.

@gvarnavi
Copy link
Member Author

Great, thanks @TomaSusi and @smribet!

This is pretty much ready on my end, with the following two caveats:

  • I don't have a windows machine to check whether the use_dill=True change for mpire works
  • Let's discuss the OBF patent issue at the meeting next week
    • I don't have strong feelings either way and happy to remove if easiest

@juliedactyl
Copy link
Contributor

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.

@kucukogluberk
Copy link

Hey everyone, sorry for the delay. I am finished with the reviews now:
First of all, I have to say the tutorials are very very good. I tried my best to find things to improve. So I am not really correcting anything, but more like suggesting improvements.

About Parallax_01.ipynb

  • Notebook works without any issues on my personal datasets ✅
  • There is a magic value at line bright_field_mask = mean_diffraction_pattern > 0.5 - for my dataset, something around 0.4 worked.
  • The values for variable selected_indices are also magical. But I think it would be such a hassle to write something that is applicable to every case.
  • At section ### Cross-correlation alignment the sentence What parallax imaging does is cross-correlate all these virtual images to the optic axis. can be improved. I suggest something like: What parallax imaging does is to cross-correlate... or more like Parallax imaging performs cross-correlation of all virtual images with respect to the optical axis.
  • At another section, within the sentence The algorithm iteratively cross-correlates the virtual BF images of succesively binned datasets (to boost signal-to-noise), using the cross-correlation shifts at the previous bin as an initial guess. we can improve what’s written in the parentheses as (to boost the signal-to-noise ratio). Also minor typo with succesively --> successively.

About Parallax_02.ipynb

  • Notebook works without any issues on my personal datasets ✅
  • Minor typo at: Here we perfom an angular average to illustrate these Fourier intensity modulations: as perfom --> perform
  • Consistent capitalization of names in sentence ...in single-particle analysis software like cryosparc or Relion to RELION and cryoSPARC
  • Minor addition: “The first thing the algorithm does is to deconvolve”
  • A question: The cross-correlation alignment in parallax_02.ipynb does not actually converge loss-wise, even though the image looks good. But we still base rest of our analysis on this reconstruction. It may not really matter, but say, if the user sees a similar convergence curve, should they proceed?

About changes from all commits in Github:
I have checked the code. I did not get down to the algorithm level, but I saw changes such as:

  • aberration_C1 ==> aberrations_C1
  • defocus_guess X
  • xp.fft.fftfreq(nx,sx).astype(np.float32)

which are very consistent with what you explain in your changes log. So it looks good.

@gvarnavi
Copy link
Member Author

Thanks @kucukogluberk and @juliedactyl!

Re: the mpire windows bug. Turns out this is rather annoying and not simply fixed by use_dill=True.

In windows the "fork" start_method for mpire is not supported, and will instead default to "spawn". This means we can't use the nice "copy-on-write" feature of mpire, making it particularly slow since each worker needs to copy the shared data. In addition to this, the data needs to be pickled to each worker, for which we need to use the dill and multiprocess packages (instead of the default multiprocessing).

I don't think this use-case warrants us adding multiprocess as a dependency to py4DSTEM, since in all my tests it was prohibitively slow anyway and the user is better off using a simple loop.

So, I added some logic to check if the system is Windows, set num_jobs=1 as the default, and warn the user to import multiprocess if they really insist in using more than one process.

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

@smribet smribet merged commit 190ba4e into dev Jan 21, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants