cabouman commented 3 years ago

We need to integrate unit tests into our package. These tests can be used to automatically test code when a pull request is done.

Perhaps we can generate a python utility that that generates synthetic sinogram data. For example, using the Shepp-Logan phantom is specified at https://en.wikipedia.org/wiki/Shepp–Logan_phantom . So a python utility can be created that generates a 3D phantom by rotating a conical section of the Shepp-Logan.

Also, we should use the pytest package for implementing the unit tests. We might also need to use something called travis.

cabouman commented 3 years ago

Wenrui, I took a look at your test code, and you have made great progress. Here are a few comments.

1) Center offset: I notice that you have set center_offset=-6 in your tests. I'm not sure why you would do this. If you just do nothing, then it will default to center_offset=0, which is the simplest and most obvious case to test. I would use the default value of center_offset in all the tests you currently have. (Later we can generate additional tests that test unusual parameter settings.)

2) Rename the tests: Your test names are things like test_01 and test_02. You should give your tests names that are understandable like 2D_recon_test and 3D_recon_test.

3) Rename phantom generator: I would change the name of generate_2D_phantom(img_size) to something like svmbir.gen_3D_SL_phantom(num_rows,num_cols). This will allow for the generation of other phantom types in the future.

4) Non-rectangular images: You need to do a 3D test with a limited angle and a non-rectangular image. I suggest you modify your image generation routines so that the input is num_rows, num_cols, and num_slices. Then one of your examples should use num_rows=256, num_cols=128, and num_slices=8 with angles ranging between -pi/3 and pi/3 . This is typical of a TEM tilt series.

5) Possibly tighter thresholds: I'm not sure if your test threshold of 0.005 is tight enough. You should find out what values are generated for say line 62, and then set the threshold be maybe 2x the observed value so the test will be sensitive to any problem with convergence.

DamonLee5 commented 3 years ago

Got it. I will update it this morning. I think I may have some problems in Non-rectangular images. If I do meet some problem, I will ask questions through email.

Best, Wenrui

On Tue, Nov 10, 2020 at 9:05 AM Charles A Bouman notifications@github.com wrote:

Wenrui, I took a look at your test code, and you have made great progress. Here are a few comments.

1.

Center offset: I notice that you have set center_offset=-6 in your tests. I'm not sure why you would do this. If you just do nothing, then it will default to center_offset=0, which is the simplest and most obvious case to test. I would use the default value of center_offset in all the tests you currently have. (Later we can generate additional tests that test unusual parameter settings.) 2.

Rename the tests: Your test names are things like test_01 and test_02. You should give your tests names that are understandable like 2D_recon_test and 3D_recon_test. 3.

Rename phantom generator: I would change the name of generate_2D_phantom(img_size) to something like svmbir.gen_3D_SL_phantom(num_rows,num_cols). This will allow for the generation of other phantom types in the future. 4.

Non-rectangular images: You need to do a 3D test with a limited angle and a non-rectangular image. I suggest you modify your image generation routines so that the input is num_rows, num_cols, and num_slices. Then one of your examples should use num_rows=256, num_cols=128, and num_slices=8 with angles ranging between -2pi/3 and 2pi/3 . This is typical of a TEM tilt series. 5.

Possibly tighter thresholds: I'm not sure if your test threshold of 0.005 is tight enough. You should find out what values are generated for say line 62, and then set the threshold be maybe 2x the observed value so the test will be sensitive to any problem with convergence.

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/cabouman/svmbir/issues/45#issuecomment-724722806, or unsubscribe https://github.com/notifications/unsubscribe-auth/AFJDA4SCGO3ZRWPMH7GOFGDSPFCDLANCNFSM4TLUBT5A .

cabouman commented 3 years ago

OK, I suggest you scale the phantom based on the smallest dimension, i.e., min(num_rows, num_cols); and you center the phantom along both axises of the image. That should be good enough for now.

DamonLee5 commented 3 years ago

Rename the tests: 'test_' is Keywords in pytest. I will make it test_2D_recon and test_3D_recon.

DamonLee5 commented 3 years ago