IMSY-DKFZ
Use same FOV definition within different reconstructions
So far all reconstruction algorithms implemented their own method to convert the field of view in mm into a field of view in pixels/voxels. To avoid errors, a single field of view conversion method is now used.
Check if flooring or rounding in the compute image dimensions function makes more sense.
Visualize the field of view grid in the create_a_custom_digital_device_twin.py plot.
Add more tests to test specific spacings, shifted fovs and ranges.
Document the rationale behind the decisions done in the algorithm.
Rounding makes more sense as it will always lead to the smaller error compared to when always flooring. Important is that the sign of the difference needs to be considered in the following computation to adjust the start and end values accordingly (subtract in the case of flooring and add in the case of ceiling).
The field of view was added as an opaque red rectangle to the PA device plot.
Further tests with different spacings to cover more test cases were added.
All changes look good to me :+1:
When running the manual tests there are differences between the reference and the test results. Especially in kwave simulations and reconstructions. This is likely due to the FOV changes, but I don't know if they are wanted like this. Perhaps double-check this before merging @kdreher Here are the reference (top) and test (bottom) results from the invision simulation test. The value ranges of the reconstruction are very different.
I can't approve the PR as I'm the original author, so you probably have to do it @kdreher
Also two manual tests failed because the adapter names which changed in #360 were not updated in another new branch #311 where the additional flags were introduced. I've now quickly fixed this bug.
All changes look good to me 👍
When running the manual tests there are differences between the reference and the test results. Especially in kwave simulations and reconstructions. This is likely due to the FOV changes, but I don't know if they are wanted like this. Perhaps double-check this before merging @kdreher Here are the reference (top) and test (bottom) results from the invision simulation test. The value ranges of the reconstruction are very different.
I can't approve the PR as I'm the original author, so you probably have to do it @kdreher
These changes are actually not related to the changes in FOV (this PR). The changes are most likely due to fixing the bug that 3D simulations were not run properly before (#251).
