Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

update ISTA step-size #39

Merged
merged 1 commit into from
Jul 5, 2024
Merged

update ISTA step-size #39

merged 1 commit into from
Jul 5, 2024

Conversation

KrisThielemans
Copy link
Member

set to .1 due to preconditioner

Now that we're using the preconditioner, the step-size should be
of order 1.
@casperdcl casperdcl added the enhancement New feature or request label Jul 5, 2024
Copy link
Member

@casperdcl casperdcl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

idk; blindly approving. @paskino?

@KrisThielemans
Copy link
Member Author

I'm going to merge this. it should work, and we'll see what happens.

@KrisThielemans KrisThielemans merged commit 85393b1 into main Jul 5, 2024
2 checks passed
@KrisThielemans KrisThielemans deleted the ISTA_step_size branch July 5, 2024 11:11
@casperdcl
Copy link
Member

casperdcl commented Jul 8, 2024

https://petric.tomography.stfc.ac.uk/leaderboard/#images&regexInput=Casper looks odd.

  • NEMA ISTA: phantom slowly disappears
  • Hoffman ISTA & BSREM: brain rotates
  • Vision BSREM: too slow to even reach 10 iterations before timeout
  • Vision ISTA: artefacts around FOV

Generally very concerned about zero-trapping from using initial=OSEM_image.

@KrisThielemans
Copy link
Member Author

https://petric.tomography.stfc.ac.uk/leaderboard/#images&regexInput=Casper looks odd.

  • NEMA ISTA: phantom slowly disappears

my guess is that the step-size for ISTA should decrease over iterations

  • Hoffman ISTA & BSREM: brain rotates

this is very weird. Possibly I generated the OSEM image with a different version, although I cannot imagine how this can happen. Luckily, the image does get sharper over iterations, so it seems a problem with the OSEM, not with the final set-up.

  • Vision BSREM: too slow to even reach 10 iterations before timeout

The default timeout is 300s. I see that on my machine I get about 6s per update. Not sure what's going on therefore. Possibly the start-up takes a bit long (but shouldn't be measured).

  • Vision ISTA: artefacts around FOV

right. if it's not an ISTA problem, it could be a problem with either the Siemens background estimate, or something that I don't know... Could solve presumably solve it by cropping (like I did for the mMR). Would have to be done soon.

Generally very concerned about zero-trapping from using initial=OSEM_image.

No idea what you're talking about.

For discussion tomorrow.

@KrisThielemans
Copy link
Member Author

I see we're using sequential views for the subsets in the example BSREM and ISTA code. That could cause some apparent rotation in the beginning.

@KrisThielemans
Copy link
Member Author

Timings of Vision files

$ ls -rt --full-time /opt/runner/logs/Casper/BSREM/Vision600_thorax/
...
-rw-r--r-- 1 ubuntu runner01        0 2024-07-08 14:02:44.991326383 +0000 warnings.txt
-rw-r--r-- 1 ubuntu runner01        0 2024-07-08 14:02:44.991326383 +0000 info.txt
-rw-r--r-- 1 ubuntu runner01        0 2024-07-08 14:02:44.991326383 +0000 errors.txt
-rw-r--r-- 1 ubuntu runner01 17036800 2024-07-08 14:06:39.464458813 +0000 iter_0000.v
-rw-r--r-- 1 ubuntu runner01      925 2024-07-08 14:06:39.464458813 +0000 iter_0000.hv
-rw-r--r-- 1 ubuntu runner01      581 2024-07-08 14:06:39.464458813 +0000 iter_0000.ahv
-rw-r--r-- 1 ubuntu runner01    18150 2024-07-08 14:08:20.599210813 +0000 events.out.tfevents.1720446732.8eea81c8f9a8
-rw-r--r-- 1 ubuntu runner01       43 2024-07-08 14:08:21.063205075 +0000 objectives.csv

so about 236s start-time (including sensitivity calculation).

for ISTA

$ ls-rt  --full-time /opt/runner/logs/Casper/ISTA/Vision600_thorax/
...
-rw-r--r-- 1 ubuntu runner01        0 2024-07-08 13:46:34.559542499 +0000 warnings.txt
-rw-r--r-- 1 ubuntu runner01        0 2024-07-08 13:46:34.559542499 +0000 info.txt
-rw-r--r-- 1 ubuntu runner01        0 2024-07-08 13:46:34.559542499 +0000 errors.txt
-rw-r--r-- 1 ubuntu runner01 17036800 2024-07-08 13:49:32.061276908 +0000 iter_0000.v
-rw-r--r-- 1 ubuntu runner01      925 2024-07-08 13:49:32.061276908 +0000 iter_0000.hv
-rw-r--r-- 1 ubuntu runner01      580 2024-07-08 13:49:32.061276908 +0000 iter_0000.ahv
-rw-r--r-- 1 ubuntu runner01 17036800 2024-07-08 13:51:02.300126320 +0000 iter_0010.v
-rw-r--r-- 1 ubuntu runner01      925 2024-07-08 13:51:02.300126320 +0000 iter_0010.hv
-rw-r--r-- 1 ubuntu runner01      580 2024-07-08 13:51:02.300126320 +0000 iter_0010.ahv
-rw-r--r-- 1 ubuntu runner01    51375 2024-07-08 13:51:57.815418798 +0000 events.out.tfevents.1720445769.d0c1e6f563a6
-rw-r--r-- 1 ubuntu runner01       98 2024-07-08 13:51:58.243413344 +0000 objectives.csv

so about 180s start-time (including sensitivity calculation) and 90s for 10 updates.

I'm not sure why the BSREM1 start-up time would be so much slower than ISTA. I haven't tested this separately.

In any case, the timeout will have to increase. (Luckily, TOF does converge faster in terms of updates).

@KrisThielemans
Copy link
Member Author

  • Vision ISTA: artefacts around FOV

right. if it's not an ISTA problem, it could be a problem with either the Siemens background estimate, or something that I don't know... Could solve presumably solve it by cropping (like I did for the mMR). Would have to be done soon.

Explained why it's not in BSREM by #44

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants