[CCPPETMR/SIRF] Selecting shared_ptr by SIRF engines. (#62)

0 views
Skip to first unread message

evgueni-ovtchinnikov

unread,
Sep 27, 2017, 6:30:17 AM9/27/17
to CCPPETMR/SIRF, Subscribed

Switched to the use of std::shared_ptr in xGadgetron and stir::shared_ptr in xSTIR. Generally, different SIRF engines can now use different shared_ptr (as specified in *_shared_ptr,h).

You can view, comment on, or merge this pull request online at:

  https://github.com/CCPPETMR/SIRF/pull/62

Commit Summary

  • added checks to pStir.ImageData methods
  • introduced pUtil.try_calling() convenience function to reduce repetitive coding
  • applied workaround that restored the printing suppression option
  • removed obsolete code from pStir.Printer
  • restored printing suppression option in Matlab
  • restored printing suppression option in Matlab
  • renamed pStir.Printer to pStir.MessageRedirector
  • renamed mStir.PrintTo to mStir.MessageRedirector
  • corrected mStir.MessageRedirector
  • added UsersGuide.md
  • tidied up pStir.py using try_calling()
  • fixed TOC bug in UsersGuide.md
  • fixed TOC bug in UsersGuide.md more thoroughly
  • tidied up pGadgetron.py using try_calling() convenience function
  • corrected Reconstructor description in UsersGuide.md
  • more information now printed by pUtil.check_status()
  • tested Python inspect.stack() as a debugging tool
  • Merge branch 'master' into devel
  • cleaned up cGadgetron to reduce warnings
  • made aDataContainer a template class so it can be used for both complex and real data
  • removed obsolete methods from MR image objects
  • started separating implementation from specification for gadgetron data containers
  • finished separating spec and implementation for gadgetron data objects (save for trivial methods)
  • separated implementation from spec in gadgetron_client
  • cleaned up gadgetron_client.h
  • cleaned up gadgetron_data_containers.h
  • moved file-specific implementation of AcquisitionData.fill to c++ AcquisitionsFile
  • cleaned up gadgetron_data_containers.h
  • moved set_acquisition_data from AcquisitionContainer to derived classes
  • safeguarded show_3D_array against wrong list of indices
  • corrected image display in mStir
  • replaced std::list with std::vector in gadgetron data container classes
  • implemented AcquisitionsVector data container (not tested)
  • switched to using AcquisitionsVector type instead of AcquisitionFile for processed MR acquisition data
  • capped the number of displayed readouts in using_acquisition_data.py
  • cloned the input data into memory in using_acquisition_data.py to accelerate processing of large data
  • tidied up MR acquisition sorting
  • corrected template parameters for std::array
  • cloned the input data into memory in using_acquisition_data.m to accelerate processing of large data
  • corrected the setting of the tile grid in show_3D_array.m
  • started developer's guide
  • more work on developer's guide
  • renamed checkExecutionStatus.m into check_status.m for consistency of function naming
  • corrected sorting functions
  • corrected template parameters for std::array
  • corrected single-chain script
  • switched xGadgetron from double to float
  • cast Inati CSMs to numpy.complex64 (from default 128)
  • switched xSTIR from double to float
  • added iutilities.h to swig extra deps
  • introduced checks for proper data type in fill methods
  • small correction for compatibility with older matlab applied
  • fixed #45 and #46
  • responded to #44
  • corrected gadgetron acquisition processor
  • started adding advanced stuff into UserGuide
  • drafted AcquisitionsContainerTemplate class to be used for selecting memory or file storage
  • continued working on drafted AcquisitionsContainerTemplate class
  • introduced class AcquisitionsInfo
  • introduced default constructors for AcquisitionsFile and AcquisitionsVector
  • tested defining storage scheme via AcquisitionsContainerTemplate
  • had to switch to AcuisitionsVector as default because of dataset closure failure on VM
  • applied a workaround to avoid dataset closure error, to be checked on VM
  • tidied up a bit
  • implemented storage scheme selection for MR acquisition data
  • redesigned MR acquisitions storage scheme management
  • some renaming done
  • moved more gadgetron data containers stuff from *.h to *.cpp
  • renamed *util* to *utilities* and cgt to cgadgetron
  • renamed +mUtil to +mUtilities
  • renamed some files in cGadgetron for consistency with cSTIR
  • Merge branch 'rename' into devel
  • replaced setup_* with set_up_*
  • Merge branch 'rename' into devel
  • put missing semicolons in simplistic_petmr.m
  • removed obsolete stuff from cstir project
  • introduced non-empty handle assertion in data containers
  • corrected Python PET scripts (pUtil->pUtilities)
  • started introducing STIR data container types
  • corrected some scripts (pUtil->pUtilities)
  • unsuccessfully tried to implement PET acquisition storage management
  • another unsuccessful attempt to design OO PET data storage management
  • introduced STIR data containers and no-OO storage management, discovered forward-projecting-to-file bug
  • re-designed PETAcquisitionData
  • implemented and tested PET acquisition data containers
  • corrected simplistic_petmr.py (pUtil->pUtilities as pUtil)
  • interfaced PET acquisition data storage selection into Python and Matlab
  • started implementing PET acquisition data algebra
  • added stir_x.cpp to cstir project
  • corrected cSTIR/CMakeLists.txt (cstir_x.cpp->stir_x.cpp
  • fixed a bug in PETAcquisitionData::same_acquisition_data()
  • implemented PETImageData container class, including algebra
  • trying to fix chrono issue
  • corrected cSTIR/CMakeLists.txt
  • added set(CMAKE_CXX_STANDARD 11) to SIRF/CMakeLists.txt
  • fixed stuff not allowed by gnu compiler
  • fixed a bug in opening non-existing file for input/output
  • trying to deal with c++11/old cmake issue
  • tidied up
  • Merge branch 'stir-data-containers' into devel
  • all cSTIR interface functions except newTextPrinter/Writer and open/closeChannel now return DataHandle* to enable exception handling
  • redirected STIR info to a file in steepest_ascent.m
  • reimplemented and tested the use of additive and background terms in PET acquisition model
  • fixed normalisation bug by clearing ProjDataFromStream::sino_stream
  • moved STIR data containers stuff to stir_data_containers.*
  • removed obsolete filename argument from PETAcquisitionModel::forward
  • implemented and tested PETAcquisitionData::dot
  • removed imaginary parts from cSTIR algebra functions arguments
  • switched to using SIRF_PATH in c2m instead of relative paths
  • made some progress in dealing with undeletable scratch files
  • fixed undeletable scratch file bug
  • redesigned PET acquisition data storage management in OO way
  • fixes #44
  • corrected SIRF version in CMakeLists.txt (fixes #57)
  • renamed pStir.py to pSTIR.py
  • renamed +mStir to +mSTIR
  • commented on storage scheme persistence in Matlab
  • Merge branch 'Stir2STIR' into devel
  • changed default step size in steepest_acsent.py to speed up demo execution
  • added more advanced stuff to UserGuide; deleted duplicate UsersGuide
  • removed SRC_PATH (Fixes #24)
  • tidied up error messages, fixes #28
  • added Visual Studio 13 project for Matlab interface generator
  • renamed mutilities.* to miutilities.* for consistency
  • implemented PET data containers algebra in Matlab
  • added argument type checks in Python
  • corrected Matlab interface (void->void*)deleteMexPrinter
  • added missing assertions to pGadgetron.py; trying using meta.class in DataContainer.m for argument check
  • corrected assertions in SIRF Python modules
  • introduced arguments type check in mGadgetron.DataContainer
  • added missing data type checks in SIRF Python modules
  • trying assert_validity function
  • tidied up argument validity checks in SIRF Python modules
  • renamed handle->handle_ in mSTIR for consistency with mGadgetron
  • introduced arguments validity checks in mGadgetron.AcquisitionModel
  • added assert_validity.m to mUtilities
  • corrected set_up_*.m (setup->set_up)
  • added another Matlab validity assertion; made assertion failure messages more useful
  • introduced argument type checks in SIRF Matlab modules
  • corrected syntax errors
  • Merge branch 'master' into devel
  • replaced boost::shared_ptr with sirf::shared_ptr
  • introduced namespaces boost_sptr (currently used) and std_sptr (to be used)
  • implemented independent definition of shared_ptr for each engine
  • removed PETImageData constructor from auto_ptr

File Changes

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

unread,
Sep 27, 2017, 8:58:44 AM9/27/17
to CCPPETMR/SIRF, Subscribed

@casperdcl do you have any idea why github claims that this PR contains so many commits? This PR is based on master, which was merged to devel recently, from which this branch was created. I guess the git merge won't be a problem, but I'd like to understand this a bit more.

Kris Thielemans

unread,
Sep 27, 2017, 9:02:23 AM9/27/17
to CCPPETMR/SIRF, Subscribed

@evgueni-ovtchinnikov I had a brief look at this. It seems a bit of a hack to me. I guess it works so I'm ok with merging it, but I'm generally quite warry of solving this stuff with #define. Also, it seems we have twice object_handle.inl (identical?) .

I'm not 100% sure, but I guess what you'd need is to have an extra template arg for your ObjectHandle class (and others?)

evgueni-ovtchinnikov

unread,
Sep 27, 2017, 9:24:00 AM9/27/17
to CCPPETMR/SIRF, Subscribed
Yes, files object_handle.inl are identical – I should have had just one of them in src/common, but I just do not know how to edit CMakeLists so that it is found.

Extra template arg would just be too much editing, with namespace it is just two lines per file.


From: Kris Thielemans [mailto:notifi...@github.com]
Sent: 27 September 2017 14:03
To: CCPPETMR/SIRF
Cc: Ovtchinnikov, Evgueni (STFC,RAL,SC); Mention
Subject: Re: [CCPPETMR/SIRF] Selecting shared_ptr by SIRF engines. (#62)


@evgueni-ovtchinnikov<https://github.com/evgueni-ovtchinnikov> I had a brief look at this. It seems a bit of a hack to me. I guess it works so I'm ok with merging it, but I'm generally quite warry of solving this stuff with #define. Also, it seems we have twice object_handle.inl (identical?) .

I'm not 100% sure, but I guess what you'd need is to have an extra template arg for your ObjectHandle class (and others?)

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<https://github.com/CCPPETMR/SIRF/pull/62#issuecomment-332513083>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AOWfVAQYdnm3XFqfdyr6DTSncRy8s1N4ks5smkdTgaJpZM4PljW0>.

evgueni-ovtchinnikov

unread,
Sep 27, 2017, 9:59:52 AM9/27/17
to CCPPETMR/SIRF, Subscribed
Could it be because I branched it from devel?

From: Kris Thielemans [mailto:notifi...@github.com]
Sent: 27 September 2017 13:59
To: CCPPETMR/SIRF
Cc: Ovtchinnikov, Evgueni (STFC,RAL,SC); Author
Subject: Re: [CCPPETMR/SIRF] Selecting shared_ptr by SIRF engines. (#62)


@casperdcl<https://github.com/casperdcl> do you have any idea why github claims that this PR contains so many commits? This PR is based on master, which was merged to devel recently, from which this branch was created. I guess the git merge won't be a problem, but I'd like to understand this a bit more.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub<https://github.com/CCPPETMR/SIRF/pull/62#issuecomment-332512149>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AOWfVO03LAGXNjtNc46f2vY897QFrXnDks5smkaBgaJpZM4PljW0>.

Kris Thielemans

unread,
Sep 27, 2017, 10:13:47 AM9/27/17
to CCPPETMR/SIRF, Subscribed

FYI, modifying CMake files:

  1. create src/common/include/SIRF/common or something
  2. in xSTIR/CMakeLists.txt do something like
include_directories(${PROJECT_SOURCE_DIR}/src/common/include)
  1. in your file
#include "SIRF/common/object_handle.inl"

I wouldn't do this right now, as we want to avoid the #define trick later (it's not good programming style, and I'm worried that we'll have a conflict at some point).

can you merge with master locally first. looks like you'll have conflicts.

evgueni-ovtchinnikov

unread,
Sep 27, 2017, 10:32:39 AM9/27/17
to CCPPETMR/SIRF, Subscribed
Merged locally, no conflicts.

From: Kris Thielemans [mailto:notifi...@github.com]
Sent: 27 September 2017 15:14
To: CCPPETMR/SIRF
Cc: Ovtchinnikov, Evgueni (STFC,RAL,SC); Mention
Subject: Re: [CCPPETMR/SIRF] Selecting shared_ptr by SIRF engines. (#62)


FYI, modifying CMake files:

1. create src/common/include/SIRF/common or something
2. in xSTIR/CMakeLists.txt do something like

include_directories(${PROJECT_SOURCE_DIR}/src/common/include)

1. in your file

#include "SIRF/common/object_handle.inl"

I wouldn't do this right now, as we want to avoid the #define trick later (it's not good programming style, and I'm worried that we'll have a conflict at some point).

can you merge with master locally first. looks like you'll have conflicts.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<https://github.com/CCPPETMR/SIRF/pull/62#issuecomment-332534757>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AOWfVDqJN12ZNEwWTUHE6h9f4xbT1CUfks5smlgYgaJpZM4PljW0>.

Kris Thielemans

unread,
Sep 27, 2017, 10:42:11 AM9/27/17
to CCPPETMR/SIRF, Subscribed

ok. please push

evgueni-ovtchinnikov

unread,
Sep 27, 2017, 10:43:36 AM9/27/17
to CCPPETMR/SIRF, Subscribed

Merged #62.

evgueni-ovtchinnikov

unread,
Sep 27, 2017, 10:43:58 AM9/27/17
to CCPPETMR/SIRF, Subscribed
Done

From: Kris Thielemans [mailto:notifi...@github.com]
Sent: 27 September 2017 15:42
To: CCPPETMR/SIRF
Cc: Ovtchinnikov, Evgueni (STFC,RAL,SC); Mention
Subject: Re: [CCPPETMR/SIRF] Selecting shared_ptr by SIRF engines. (#62)


ok. please push

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<https://github.com/CCPPETMR/SIRF/pull/62#issuecomment-332544178>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AOWfVKau1i5nZrgomx852Pq6EpHa-2Zdks5sml7BgaJpZM4PljW0>.

evgueni-ovtchinnikov

unread,
Sep 27, 2017, 10:48:49 AM9/27/17
to CCPPETMR/SIRF, Subscribed

Merged manually

Reply all
Reply to author
Forward
0 new messages