Scale the FPM object plot by its real-space pixel size, not dxo - #97
Open
tbvanderwoude wants to merge 3 commits into
Open
Scale the FPM object plot by its real-space pixel size, not dxo#97tbvanderwoude wants to merge 3 commits into
tbvanderwoude wants to merge 3 commits into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
While running the FPM tutorial I noticed an apparent mistake in the field of view of the object estimate that is plotted
Assuming my calculations are correct, the per-frame field of view is only 288.63 µm, using the detector pixel size of 3.45 um with magnification 1.53 and frame size 128 x 128 px. For FPM, the FOV should remain the same so that this FOV is also displayed in the estimate plot.
What changed
The changes are rather straightforward:
dxo_fpm(real-space object size for FPM) andLo_fpm(single-frame FOV for FPM) were added toReconstructionMonitor,updateObjectProbeErrorMonitorwas changed to use a new propertyobjectPixelSizewhich selectsdxofor CPM anddxo_fpmfor FPM. This ensures the correct physical FOV is displayed also for FPM.Tests
This PR adds 3 tests, two of which (
test_fpm_object_sampling_preserves_field_of_view,test_monitor_plots_fpm_object_with_fpm_pixel_size) verify correct new behavior for FPM, with the other one (test_monitor_plots_cpm_object_with_dxo) a simple regression test for CPM to ensure this behavior is still valid.Note
I could not find a CONTRIBUTING.md or equivalent so I tried to follow the style of #96 and previous PR's as much as possible. Please let me know if anything is missing that is required to evaluate this change.
I also noticed that for FPM the estimate plot appears to show a pupil but it says 'probe' and has real-space axes. I did not change this but to make this Monitor plot fully correct for FPM this should also be changed.