[CCPPETMR/SIRF] Tof (#390)
Alexander Whitehead
Partially addresses #315
You can view, comment on, or merge this pull request online at:
https://github.com/CCPPETMR/SIRF/pull/390
Commit Summary
- added tof to STIR.py PETAcquisitionData.show() has sinogram selection. Would need TOF bin selection.
- pass through get num tof bins from stir
- Merge branch 'master' of https://github.com/CCPPETMR/SIRF into tof
File Changes
- M src/xSTIR/cSTIR/cstir.cpp (26)
- M src/xSTIR/cSTIR/include/sirf/STIR/stir_data_containers.h (2)
- M src/xSTIR/cSTIR/stir_data_containers.cpp (17)
- M src/xSTIR/pSTIR/STIR.py (38)
Patch Links:
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.
Kris Thielemans
thanks. Ideally, you modify the travis.yml to use the SuperBuild with appropriate STIR_TAG and STIR_URL. otherwise, please add [ci skip] to commit messages, as we'd get a lot of failures...
Alexander Whitehead
@ALEXJAZZ008008 pushed 1 commit.
- 72e42db force ALEXJAZZ008008 repo and force TOF branch in travis
—
You are receiving this because you are subscribed to this thread.
Alexander Whitehead
Closed #390.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.
Kris Thielemans
why closed?
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.
Alexander Whitehead
This was accidental, sorry. I wanted to use the name TOF for a different branch. I still have the changes, I'll deal with the pull request
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.
Alexander Whitehead
Reopened #390.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.
Kris Thielemans
@KrisThielemans requested changes on this pull request.
I might have found a problem since my STIR commit UCL/STIR@3f4c94d. Possibly my choice for get_num_sinograms to now include TOF bins wasn't all that great...
> - dim[0] = sptr_ad->get_num_tangential_poss();
- dim[1] = sptr_ad->get_num_views();
- dim[2] = sptr_ad->get_num_sinograms();
- dim[3] = sptr_ad->get_num_TOF_bins();
- return (void*)new DataHandle;
- }
- CATCH;
+ try
+ {
+ int* dim = (int*) ptr_dim;
+
+ SPTR_FROM_HANDLE(PETAcquisitionData, sptr_ad, ptr_acq);
+
+ dim[0] = sptr_ad->get_num_tangential_poss();
+ dim[1] = sptr_ad->get_num_views();
+ dim[2] = sptr_ad->get_num_sinograms();
⬇️ Suggested change
- dim[2] = sptr_ad->get_num_sinograms(); + dim[2] = sptr_ad->get_num_non_tof_sinograms();
In src/xSTIR/cSTIR/stir_data_containers.cpp:
> @@ -33,26 +33,33 @@ using namespace sirf; std::string PETAcquisitionData::_storage_scheme; shared_ptr<PETAcquisitionData> PETAcquisitionData::_template; -float -PETAcquisitionData::norm() const +float PETAcquisitionData::norm() const
revert
In src/xSTIR/cSTIR/stir_data_containers.cpp:
> +
+ for (seg_iter = seg.begin_all(); seg_iter != seg.end_all();)
+ {
revert. no code clean-up in a "new feature PR"
In src/xSTIR/cSTIR/stir_data_containers.cpp:
> +
+ if (s != 0)
+ {
revert
In src/xSTIR/cSTIR/stir_data_containers.cpp:
> +
+ for (seg_iter = seg.begin_all(); seg_iter != seg.end_all();)
+ {
revert
In src/xSTIR/cSTIR/stir_data_containers.cpp:
> double r = *seg_iter++; t += r*r; } } } +
revert
> + + #if ns >= 16: + #tiles = (4, 4) + #else: + #tiles = None +
I'm not sure why you did this, but I'd rather not do a surprising change in this PR
> + #err = show_3D_array(data[0,:,:,:], \ + # index = sino[f : t], tile_shape = tiles, \ + # label = 'sinogram', \ + # xlabel = 'tang.pos', ylabel = 'view', \ + # suptitle = title, show = (t == ns)) +
why commented out?
> + def set_MAP_model(self, model): + parms.set_char_par\ + (self.handle, self.name, 'MAP_model', model) + def set_maximum_relative_change(self, value): + parms.set_float_par\ + (self.handle, self.name, 'set_maximum_relative_change', value) + def set_minimum_relative_change(self, value): + parms.set_float_par\ + (self.handle, self.name, 'set_minimum_relative_change', value) + def get_objective_function(self): + obj_fun = PoissonLogLikelihoodWithLinearModelForMean() + obj_fun.handle = pystir.cSTIR_parameter\ + (self.handle, self.name, 'objective_function') + check_status(obj_fun.handle) + return obj_fun
not sure why this is here, maybe it needs another merge from master?
In src/xSTIR/cSTIR/cstir_p.cpp:
> + if (boost::iequals(name, "set_maximum_relative_change")) + recon.set_maximum_relative_change(dataFromHandle<double>((void*)hv)); + if (boost::iequals(name, "set_minimum_relative_change")) + recon.set_minimum_relative_change(dataFromHandle<double>((void*)hv));
seems a good addition, but shouldn't be a in a TOF PR
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.
Alexander Whitehead
Closed #390.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.