Scatter estimation script

robbietuk commented 4 years ago

Initial PR for the scatter estimation and adds all the parameter files.

Currently the script works but linking to the par files may to be made relative.

Other than that, the script appears to work and it runs for my data on the cluster.

To do:

[x] Resolve the scatter par files issue. This can just link to STIR's examples
[x] Scatter estimation uses some reconstruction parameters, these are currently tuned to the D690 (number of subsets). Can we make these general? Support for other scanners is supported through the optional arguments.

robbietuk commented 4 years ago

Changes to STIR scatter estimation may affect this PR (https://github.com/UCL/STIR/pull/744)

KrisThielemans commented 4 years ago

Changes to STIR scatter estimation may affect this PR (UCL/STIR#744)

shouldn't, as far as I know. Those changes should be backwards compatible for any user. It would be safer to point to the STIR example files (as they will always work), but we don't have a nice way for that at the moment. We need to export those anyway. https://github.com/UCL/STIR/issues/745

robbietuk commented 3 years ago

I think I merge this and imediately add an issue relating to the number of subsets in the scatter simulation will only work if the scanner can handle 18 subsets (i.e. D690 and not mMR)

robbietuk commented 3 years ago

I'd prefer that you use files from the STIR installed examples (as they are there now). could do that later of course

Yes I agree. I will need to check how to automatically find them, preferably without another parameter parsed.

KrisThielemans commented 3 years ago

I'd prefer that you use files from the STIR installed examples (as they are there now). could do that later of course

Yes I agree. I will need to check how to automatically find them, preferably without another parameter parsed.

@danieldeidda is working on this in https://github.com/UCL/STIR/pull/801, but for the moment, you could do something like

STIR_install_dir=$(dirname $(dirname $(command -v stir_math)))

robbietuk commented 3 years ago

STIR_install_dir=$(dirname $(dirname $(command -v stir_math)))

Done. I am fairly happy with the cleanup. I added additional documentation because the STIR scatter code can be very complex and this will make it simpler. I will check this over in a few days for final corrections before it is merged.

robbietuk commented 3 years ago

[...]why do we flip the GATE image here? It would be better to do it as part of the GATE sim itself I think.

From #48, GATE .hdr/.img images output by GATE are reflected in z, not sure why. We need to flip here to correct for this. Including these corrections after the GATE simulation call is possible and should be addressed. A lot is done here to work around this issue.

Also, this computes ACFs already as well of course.

Yes, I am trying to figure out how to deal with this. The ACF code is currently hard coded. Should we have two output arguments, e.g. additive filename and multfactors filename

robbietuk commented 3 years ago

I think I am going to merge, I have run the script many times and I seem to get a meaningful image out. After the merge, I will create new little issues for each of the aforementioned problems, i.e.

The script actually compute the acf/multfactors too
Flipping the GATE images

@KrisThielemans Any minor comments/corrections? I will merge tomorrow I think.

KrisThielemans commented 3 years ago

go ahead.

UCL / STIR-GATE-Connection

Scatter estimation script #50