[SyneRBI/SIRF] Trying to accelerate PET algebra tests for larger data (PR #1027)
Evgueni Ovtchinnikov
trying to fix #1013 for larger data
You can view, comment on, or merge this pull request online at:
https://github.com/SyneRBI/SIRF/pull/1027
Commit Summary
- 0565a50 trying to accelerate tests for larger data
File Changes
(1 file)
Patch Links:
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.
Kris Thielemans
@KrisThielemans commented on this pull request.
looks good. I didn't check current run-time.
Where does tests/data.hs sit? Is it checked in ? (I didn't check)
In src/xSTIR/pSTIR/tests/test_algebra.py:
> def setUp(self):
pet.AcquisitionData.set_storage_scheme('file')
- path = os.path.join(
- examples_data_path('PET'), 'thorax_single_slice', 'template_sinogram.hs')
+ sirf_path = os.environ.get('SIRF_PATH')
+ data_path = os.path.join(sirf_path, 'tests', 'data.hs')
+ if os.path.exists(data_path):
+ print('reading data from %s' % data_path)
+ data = pet.AcquisitionData(data_path)
+ self.image1 = data.get_uniform_copy(0)
+ self.image2 = data.get_uniform_copy(0)
+ return
+ path = os.path.join(examples_data_path('PET'),
not used?
Edoardo Pasca
I think setUp should just rebin the data as you propose:
self.set_storage_scheme()
sirf_path = os.environ.get('SIRF_PATH')
path = os.path.join(examples_data_path('PET'),
'mMR', 'mMR_template_span11_small.hs')
template = pet.AcquisitionData(path)
# print(template.dimensions())
data = template.get_uniform_copy(0)
data = data.rebin(3, num_views_to_combine=6, num_tang_poss_to_trim=200)
# print('rebinned to ', data.dimensions())
self.image1 = data
self.image2 = data.get_uniform_copy(0)
Also the file data.hs that you create the first time you run the unit test (actually setUp) should be deleted at the end of the test, during tearDown, making the trick not really viable.
Edoardo Pasca
Evgueni Ovtchinnikov
@evgueni-ovtchinnikov commented on this pull request.
In src/xSTIR/pSTIR/tests/test_algebra.py:
> def setUp(self):
pet.AcquisitionData.set_storage_scheme('file')
- path = os.path.join(
- examples_data_path('PET'), 'thorax_single_slice', 'template_sinogram.hs')
+ sirf_path = os.environ.get('SIRF_PATH')
+ data_path = os.path.join(sirf_path, 'tests', 'data.hs')
+ if os.path.exists(data_path):
+ print('reading data from %s' % data_path)
+ data = pet.AcquisitionData(data_path)
+ self.image1 = data.get_uniform_copy(0)
+ self.image2 = data.get_uniform_copy(0)
+ return
+ path = os.path.join(examples_data_path('PET'),
looks good. I didn't check current run-time.
Where does tests/data.hs sit? Is it checked in ? (I didn't check)
testing time is now 14 sec
data.hs and data.s are created, if not present, in SIRF/tests by the first test - do not know how to delete them after all tests finished (CMake can, I presume?)
not used?
sorry, do not get what you refer to
Evgueni Ovtchinnikov
I think setUp should just rebin the data
rebinning in each test takes twice as long (30 sec vs 14 sec)
admittedly my fix is quite ugly, but I do not understand how unittest works, so cannot suggest anything else at the moment
Edoardo Pasca
Each unit tests will be preceded by a call to setUp and call to tearDown. The idea here is to create whatever temporary thing required for the test and clear it away even if the test fails.
Your fix is functional, but does go against the whole idea of setUp and tearDown. Why not add small PET AcquisitionData and ImageData files in the repository?
Edoardo Pasca
The file created during the first unittest will end up in ${SIRF_PATH}/tests, which is in the source directory. Does this mean that git will not like it when running a second time?
If so, I believe we should delete the data.hs file once done with the tests. I'm not so sure if there is a single test class created to keep the count of how many tests have been run.
A possible solution is to be pragmatic and think that this file will be used by the PET Python algebra only and we can create it in the build directory so that we reduce the pollution of the source and install directories to a minimum. I'd be in favour of this solution.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.
Edoardo Pasca
I suggest to close this as issue has been fixed by using a smaller template for the test data.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Kris Thielemans
fine for me
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Edoardo Pasca
Closed #1027.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Edoardo Pasca
Closing as completed by bb1c437
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.