API: Resolve position correction and field of view padding

[carterbox]

Purpose

With the altered single-mode-only operators the implementation of Dwivedi's position correction algorithm needs to be fixed, but also we need to implement some explicit checking for probe positions that are out of bounds.

Approach

• [x] In debug mode, the operators should raise an assertion error for any positions that cause illegal memory access.
• [x] In all modes, at the start of each iteration, we should always check that the positions are strictly within a 1 pixel buffer of the edges of the object because this buffer is required for interpolation and for position updates.
• [x] Automatically add determine the field of view based on scan when object is not provided

Pre-Merge Checklists

Submitter

• [x] Write a helpfully descriptive pull request title.
• [x] Organize changes into logically grouped commits with descriptive commit messages.
• [x] Document all new functions.
• [x] Write tests for new functions or explain why they are not needed.
• [x] Build the documentation successfully
• [x] Use yapf to format python code.

Reviewer

• [x] Actually read all of the code.
• [ ] Run the new code yourself.
• [ ] Write a summary of the changes as you understand them.
• [ ] Thank the submitter.

[pep8speaks]

Hello @carterbox! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers:

Comment last updated at 2020-05-07 16:21:19 UTC

[nikitinvv]

I would suggest making additional padding of psi at the beginning of reconstruction. One can add many extra pixels, e.g. psi[1].shape/8,psi[2].shape/8 to each side of psi and never have problems with positions. This will not affect performance.

[nikitinvv]

'Check allowed position after position update' means ignoring the data related to the positions that are out of field of view, or means extending the field of view? Both cases are bad.

[carterbox]

Both cases are bad.

@nikitinvv, From our discussions offline, I thought we decided that the operators should not ignore positions outside the field of view (illegal positions) and instead should raise an error if illegal positions were used. Thus, check_allowed_positions() only raises an error that is more helpful than an unexplained indexing error from the Ptycho operator; it doesn't alter any arrays.

One can add many extra pixels, e.g. psi[1].shape/8,psi[2].shape/8 to each side of psi and never have problems with positions.

That makes sense. The question is whether we should (1) always pad the object automatically or (2) pad only if the user provides no initial guess for the object. I think (2) is a better choice; we could auto-initialize with something like np.ones(field_of_view * (1 + 1 / 8)). What do you think?

[carterbox]

Increasing the size of psi affects the tomography problem because it increases the projection volume?

[nikitinvv]

@nikitinvv, From our discussions offline, I thought we decided that the operators should not ignore positions outside the field of view (illegal positions) and instead should raise an error if illegal positions were used. Thus, check_allowed_positions() only raises an error that is more helpful than an unexplained indexing error from the Ptycho operator; it doesn't alter any arrays.

Ok, we can keep in this developing mode.

That makes sense. The question is whether we should (1) always pad the object automatically or (2) pad only if the user provides no initial guess for the object. I think (2) is a better choice; we could auto-initialize with something like np.ones(field_of_view * (1 + 1 / 8)). What do you think?

Agree

Increasing the size of psi affects the tomography problem because it increases the projection volume?

This is true. But in tomo we typically do padding as well. I hope 1/8 should affect performance that much

[carterbox]

I think we should choose the padding depending on the probe size not on the width of the scan area. This is because intuitively, covering a larger area with a scanning trajectory should not increase the uncertainty of the scan positions? I suggest that we pad with one width of the probe? Or like a half-width of a probe? @YudongYao, what do you think?

[nikitinvv]

@carterbox In 3d cases one has to realign projections, i.e. perform some shifts. These shifts can be bigger than the probe size.. Then tomography will work faster with sizes of powers of 2.. You can do 1 probe padding if you want or ask users to do this. This is not a problem in practice, please dont spend much time thinking about this.

[YudongYao]

@carterbox If I understand this correctly, the reason you need padding is because during the position correction, some scan positions may go out of the defined field of view. For this purpose, I think it doesn't matter if you pad with the width of the probe or the scan area.

[carterbox]

Thanks, @nikitinvv @YudongYao!