cabouman / mbircone

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

detector row offset calculation #124

Closed dyang37 closed 1 year ago

dyang37 commented 1 year ago

This pull request fixes the det_row_offset calculation in NSI preprocess function. The fix is only one line of code :-)

Results of demo_nsi_preprocess.py before & after the fix:

before (det_row_offset=0): recon_coronal187

after (det_row_offset=1.15): recon_coronal187

The results are visually similar before & after because the row offset is small.

Also tested the code with the tin-filtered dataset, which has a larger row_offset=16.76. It yields a clear reconstruction without misalignment artifacts.

dyang37 commented 1 year ago

Tested demo_3D_shepp_logan.py and demo_mace3D_fast.py, both ran as expected.

This pull request changes one line in NSI_load_scans_and_params which is only used by demo_nsi_preprocess.py, it does not affect anything else -> seems safe.

Thanks Marco! When you have time, could you please also help me test demo_nsi_preprocess.py? The change in this PR is specific to the geometry calculation of NSI datasets. Therefore only the script demo_nsi_preprocess.py will get affected.

Thanks!

bommaritom commented 1 year ago

OK. I started running it now, it looks like it will take a while. Will let you know once it's finished.

bommaritom commented 1 year ago

Ok. Ran successfully in a bit over an hour.

dyang37 commented 1 year ago

Thanks Marco! The run time is as expected, given the size of the reconstruction problem (480x384x384). Regards, Diyu


From: Marco Bommarito @.> Sent: Friday, March 24, 2023 4:08 PM To: cabouman/mbircone @.> Cc: Yang, Diyu @.>; Author @.> Subject: Re: [cabouman/mbircone] detector row offset calculation (PR #124)

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

Ok. Ran successfully in a bit over an hour.

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

cabouman commented 1 year ago

OK, since Marco tested this, and it takes an hour to run, and it is only one line of code, I'm accepting the PR.