Ror dev

[bommaritom]

Changelog:

Changes to recon.py:

• compute_img_params is now auto_img_params (line 231), which allocates imageparams dict for recon and for project.
• Retain the two helper functions corresponding to dicts (auto_img_params, compute_sino_params). These exist because mbircone relies on dicts to pass parameters to the C (in contrast, svmbir does not).
• Add auto_image_size (see lines 209-229) to handle computations specific to recon. This follows the example in svmbir.
• Function signature for recon is changed: variables are reordered to match svmbir; add the four additional “region” parameters.

Changes to other modules:

• Add more flexibility to phantom.py: can now scale down phantom within the image and/or translate off-center
• Update function calls in mace.py to reflect changed recon signatures. Remove reference to compute_img_size; because of this, init_image CAN NO LONGER be a scalar (no way to set an image size). Someone should verify to make sure this is acceptable. I think it is OK because mace4D also does not use scalar values for init_image.
• Geometry in C function isInsideMask in MBIRModularUtilities3D.c is changed from an oval to a circle

Demo files:

• Update demo_3D_shepp_logan.py to reflect new changes
• demo_3D_shepp_logan_variable_pixel_pitch.py is now called demo_3D_shepp_logan_test.py, tests various types of reconstruction (variable pixel pitch, off-axis phantom)

Note: All the demo files have run so far, but I would expect there to be a few bugs. Maybe run a few thorough test runs.

[cabouman]

I just tested this, and it seems to work. Diyu, can you please test this?

[dyang37]

Tested both 3D Shepp Logan and MACE demos, and they work fine. However, I have some questions regarding the demo scripts:

1. We changed magnification from 2.0 to 1.0 in both demo_3D_shepp_logan.py and demo_3D_shepp_logan_test.py. Is this intended (to simulate parallel beam behavior)?
2. We also hardcoded delta_pixel_recon to be 1.0. Given that we set up magnification=1.0, this matches up with the default value of delta_pixel_image=delta_pixel_detector/magnification. So should we just delete this variable and use the default value instead (except for the variable pixel pitch case)?
3. The phantom & recon for the off axis case seems a bit pixelated:

Perhaps it's better to generate a higher resolution phantom, given that the effective object region is small compared to the entire phantom size in the off axis case?

[bommaritom]

Ok, I'm a bit nervous that there weren't any bugs but I guess it's a good thing.

1. There was no thought process behind this for me, do you think it would be better with magnification=2.0 (and then we would also change the detector pixel size)?
2. I added that line "just in case" we wanted to mess with the values; I think it just happens to line up with what the default value would be.
3. Yes, I can adjust the resolution. I have noticed that with too-low pixel pitch we lose continuity with some phantom features. The issue is that it takes forever to run, but it is probably fine. So I will double the resolution in that reconstruction.

[dyang37]

Ok, I'm a bit nervous that there weren't any bugs but I guess it's a good thing.

1. There was no thought process behind this for me, do you think it would be better with magnification=2.0 (and then we would also change the detector pixel size)?
2. I added that line "just in case" we wanted to mess with the values; I think it just happens to line up with what the default value would be.
3. Yes, I can adjust the resolution. I have noticed that with too-low pixel pitch we lose continuity with some phantom features. The issue is that it takes forever to run, but it is probably fine. So I will double the resolution in that reconstruction.

Thanks Marco. magnification=1.0 is supported by the software, but I don't think it's realistic in reality (unless the detector is infinitely far away from the source). So I'd say we just stick to the old magnification value unless there's any difference in terms of reconstruction quality with different magnifications. I don't think we need to change delta_pixel_detector since we are using ALU, but you might want to change delta_pixel_recon to be 1/magnification (or simply remove it and let the function use its default delta_pixel_image value). Please let me know if you need me to modify/test anything. Everything else looks good. Diyu

[bommaritom]

Hi all. Can Profs. Bouman & Buzzard give the go-ahead for this pull request?

[bommaritom]

Ok never mind, I see Prof. Bouman's comment from earlier.

[cabouman]

I notice there were updates to the demo files. Have you tested everything?

[cabouman]

Marco, good job on this, but in the future, don't accept your own PR. It's dangerous.

[dyang37]

Thanks Marco for getting this implemented and tested! I might take a further look into the demo script, and perhaps open another PR and make some (very slight) modifications, if necessary.

I noticed that delta_pixel_image is set to 0.5 in the demo. Since we are using ALU, I don't think it makes any difference between delta_pixel_image=0.5 or 1 (the default value). Therefore, I would prefer to stick to the default value (i.e. simply remove delta_pixel_recon and delta_pixel_phantom from the demo, just to improve the readability of the code.

I've tested the code before the modification, so I'm sure it's going to run without issues 🙂

Regards, Diyu

---

From: Charles A Bouman @.> Sent: Wednesday, September 21, 2022 11:40 AM To: cabouman/mbircone @.> Cc: Diyu Yang @.>; Review requested @.> Subject: Re: [cabouman/mbircone] Ror dev (PR #93)

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

Marco, good job on this, but in the future, don't accept your own PR. It's dangerous.

— Reply to this email directly, view it on GitHubhttps://github.com/cabouman/mbircone/pull/93#issuecomment-1253887548, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AB2Q7OAYF5FLTPOVTNJXAO3V7MT7XANCNFSM6AAAAAAQQNIQZU. You are receiving this because your review was requested.Message ID: @.***>

[dyang37]

There might be an issue for the off axis recon in demo_3D_shepp_logan_test.py, potentially because the object of interest is too close to the edge of the full ROR region. I'm getting artifacts in the coronal slice recon: Phantom image:

Off-axis recon:

This seems to be a result that the object of interest is too close to the edge of the full ROR region. Moving the object down a little bit (changing offset_z from 0.12 to 0.10) results in the following phantom and off-axis reconstruction:

Off-axis recon:

[bommaritom]

Ok. Truth be told I got a little bogged down with the demo file spaghetti, hope it is all sensible.

Also sorry for accepting my own PR, I saw the green button and thought it meant you all had approved... :[