search

cabouman / mbircone

BSD 3-Clause "New" or "Revised" License
11 stars 9 forks source link

Background offset #129

Closed dyang37 closed 1 year ago

dyang37 commented 1 year ago

This PR adds the background offset calculation functionality into preprocess module. There is also a test script: test/test_background_offset.py.

cabouman commented 1 year ago

I fixed a compiler warning by changing like 112 of interface.c to have the correct type conversion: for(i=0; i<(int)img.params.N_ximg.params.N_yimg.params.N_z; i++)

cabouman commented 1 year ago

This is good, but here are some changes to make:

  1. I think you should take the median rather than the mean of the views. That way if the object rotates out of the field of view for a few views, that won't screw everything up. So the line sino_mean=np.mean(sino, axis=0) needs to change.

  2. The test script is useful, but it needs some improvements. a) The test script should test with the default value of edge_width rather than setting it. b) The displayed image from the test script seems to have multiple lines on the top and the left, but not on the right. You should only put a red line at the column = edge_width; column=width-edge_width, and row = edge_width. c) The script doesn't give any visual indication that the estimated background offset is correct. Perhaps you can display a view with the offset removed? Or you can do a quick reconstruction from this data to show that the resulting reconstruction does not have artifacts?

  3. When you see a simple compiler warning like a type mismatch, fix the C code and do a PR. I did such a fix. I think I saw other compiler warnings. This isn't a huge priority, but it only takes a few minutes to fix and test a type mismatch. Do them when you see them.

  4. When you have made these corrections/changes, let me know and I will relook at your PR.

dyang37 commented 1 year ago

Thanks Dr. Bouman. I'll make the changes as you suggested. On top of that, according to the Lilly meeting today, there are also several things that we might need to discuss and modify:

  1. For the testing script of the background offset correction algorithm, we can provide a quick test and a thorough test.

    • For the quick test, the purpose is to visually check the background intensity to make sure that it is closer to zero after the correction. Other than the modifications you suggested, we should also mark the sinorgam pixels with negative values.
    • For the thorough test, we should perform two recons with and without background offset correction, and display the results.

  2. Please let me know if this plan looks good.

  3. We observed that sometimes the detector channel on the outer edge contain anomaly values (slide #5 of today's slides). According to Craig, it could be that there are no photon measurements on the edge channel. Craig said he will help us verify this information, and see if there is a way to know which detector channels are not measured. This means that we probably need exclude the outmost edge channel when selecting the background areas. Please let me know how you would like to handle this case.

I'll close the PR for now, make the modifications according to your suggestions, and let you know once I'm done.

Regards, Diyu

From: Charles A Bouman @.> Sent: Thursday, May 18, 2023 8:16 AM To: cabouman/mbircone @.> Cc: Yang, Diyu @.>; Author @.> Subject: Re: [cabouman/mbircone] Background offset (PR #129)

---- External Email: Use caution with attachments, links, or sharing data ----

This is good, but here are some changes to make:

  1. I think you should take the median rather than the mean of the views. That way if the object rotates out of the field of view for a few views, that won't screw everything up. So the line sino_mean=np.mean(sino, axis=0) needs to change.

  2. The test script is useful, but it needs some improvements. a) The test script should test with the default value of edge_width rather than setting it. b) The displayed image from the test script seems to have multiple lines on the top and the left, but not on the right. You should only put a red line at the column = edge_width; column=width-edge_width, and row = edge_width. c) The script doesn't give any visual indication that the estimated background offset is correct. Perhaps you can display a view with the offset removed? Or you can do a quick reconstruction from this data to show that the resulting reconstruction does not have artifacts?

  3. When you see a simple compiler warning like a type mismatch, fix the C code and do a PR. I did such a fix. I think I saw other compiler warnings. This isn't a huge priority, but it only takes a few minutes to fix and test a type mismatch. Do them when you see them.

  4. When you have made these corrections/changes, let me know and I will relook at your PR.

— Reply to this email directly, view it on GitHubhttps://github.com/cabouman/mbircone/pull/129#issuecomment-1552968716, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AB2Q7OERSN67WE6OOIT35FDXGYHKBANCNFSM6AAAAAAYF473XE. You are receiving this because you authored the thread.Message ID: @.***>

dyang37 commented 1 year ago

Closing this for now. Will reopen after I finish with the modifications as discussed.