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

Parallelproj old OpenMP fix #1247

Merged
merged 6 commits into from
Sep 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 16 additions & 1 deletion .appveyor.yml
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,30 @@ build_script:
# find boost on Appveyor. Version depends on VM
- for /D %%d in (C:\Libraries\boost_*) do set BOOST_ROOT=%%d
- echo Using Boost %BOOST_ROOT%
# find miniconda
- for /D %%d in (C:\Miniconda*-x64) do set MINICONDA=%%d
- echo Using Miniconda %MINICONDA%
- "set PATH=%MINICONDA%;%MINICONDA%\\Scripts;%MINICONDA%\\Library\\bin;%PATH%"
# install parallelproj and Python stuff
- conda install -c conda-forge -yq libparallelproj swig numpy pytest
- CALL conda.bat activate base
- python --version
- mkdir build
- mkdir install
- cd build
- cmake.exe .. -DCMAKE_INSTALL_PREFIX="C:\projects\stir\install" -DCMAKE_BUILD_TYPE=%CONFIGURATION% -DCMAKE_CONFIGURATION_TYPES=%CONFIGURATION% -DSTIR_OPENMP:BOOL=ON -DBUILD_DOCUMENTATION:BOOL=OFF
- cmake.exe .. -A x64 -DCMAKE_INSTALL_PREFIX="C:\projects\stir\install" -DCMAKE_BUILD_TYPE=%CONFIGURATION% -DCMAKE_CONFIGURATION_TYPES=%CONFIGURATION% -DSTIR_OPENMP:BOOL=ON -DBUILD_DOCUMENTATION:BOOL=OFF -DBUILD_SWIG_PYTHON:BOOL=ON -DPython_EXECUTABLE="%MINICONDA%\\python.exe"
- cmake.exe --build . --config %CONFIGURATION%
- cmake.exe --build . --target install --config %CONFIGURATION%
- conda deactivate

test_script:
- cd C:\projects\stir\build
- CALL conda.bat activate base
- python --version
- ctest --output-on-failure -C %CONFIGURATION%
- cd ..\recon_test_pack
- run_tests --nointbp "C:\projects\stir\install\bin\"
- cd ..\src
- "set PYTHONPATH=C:\\projects\\stir\\install\\python"
- python -m pytest .
- conda deactivate
10 changes: 5 additions & 5 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@
# See STIR/LICENSE.txt for details

# cmake file for building STIR. See the STIR User's Guide and http://www.cmake.org.
cmake_minimum_required(VERSION 3.1.0)


# avoid warning about WIN32 no longer defined in CYGWIN
set(CMAKE_LEGACY_CYGWIN_WIN32 0)
if (CMAKE_HOST_WIN32)
cmake_minimum_required(VERSION 3.12.0)
else()
cmake_minimum_required(VERSION 3.1.0)
endif()

# enable ccache https://ccache.samba.org/
find_program(CCACHE_PROGRAM ccache)
Expand Down
1 change: 1 addition & 0 deletions documentation/release_5.2.htm
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ <h3>Deprecated functionality</h3>

<h3>Build system and dependencies</h3>
<ul>
<li>CMake 3.12 is now required on Windows. (It will be required on all platforms in the next version).</li>
<li>We now use CMake's <a href=https://gitlab.kitware.com/cmake/community/-/wikis/doc/tutorials/Object-Library>OBJECT library feature</a> for the registries. This avoids re-compilation of the registries for every executable and therefore speeds-up building time. Use of STIR in an external project is not affected as long as the recommended practice was followed. This is now documented in the User's Guide.
<br /><a href="https://github.com/UCL/STIR/pull/1141/">PR #1141</a>.
</li>
Expand Down
91 changes: 39 additions & 52 deletions src/recon_buildblock/Parallelproj_projector/ParallelprojHelper.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,22 @@ detail::ParallelprojHelper::ParallelprojHelper(const ProjDataInfo& p_info, const
#ifdef STIR_OPENMP
// Using too many threads is counterproductive according to my timings, so I limited to 8 (not necessarily optimal!).
const auto num_threads_to_use = std::min(8,get_max_num_threads());
# if _OPENMP >=201012
# define ATOMICWRITE _Pragma("omp atomic write") \

# define CRITICALSECTION
# else
# define ATOMICWRITE
# if defined(_MSC_VER) && (_MSC_VER < 1910)
// no _Pragma until VS 2017
# define CRITICALSECTION
# else
# define CRITICALSECTION _Pragma("omp critical(PARALLELPROJHELPER_INIT)")
# endif
# endif
#else
# define ATOMICWRITE
# define CRITICALSECTION
#endif
for (int seg : segment_sequence)
{
Expand All @@ -97,66 +113,37 @@ detail::ParallelprojHelper::ParallelprojHelper(const ProjDataInfo& p_info, const
bin.view_num() = view_num;
bin.tangential_pos_num() = tangential_pos_num;
// compute index for this bin (independent of multi-threading)
std::size_t this_index = index + (bin.tangential_pos_num() - p_info.get_min_tangential_pos_num())*3;
const std::size_t this_index = index + (bin.tangential_pos_num() - p_info.get_min_tangential_pos_num())*3;
LORInAxialAndNoArcCorrSinogramCoordinates<float> lor;
LORAs2Points<float> lor_points;

p_info.get_LOR(lor, bin);
if (lor.get_intersections_with_cylinder(lor_points, radius) == Succeeded::no)
{ // memory is already allocated, so just passing in points that will produce nothing
#ifdef STIR_OPENMP
#pragma omp atomic write
#endif
xstart[this_index] = 0;
#ifdef STIR_OPENMP
#pragma omp atomic write
#endif
xend[this_index] = 0;
#ifdef STIR_OPENMP
#pragma omp atomic write
#endif
xstart[this_index+1] = 0;
#ifdef STIR_OPENMP
#pragma omp atomic write
#endif
xend[this_index+1] = 0;
#ifdef STIR_OPENMP
#pragma omp atomic write
#endif
xstart[this_index+2] = 0;
#ifdef STIR_OPENMP
#pragma omp atomic write
#endif
xend[this_index+2] = 0;
{
// memory is already allocated, so just passing in points that will produce nothing
CRITICALSECTION
{
ATOMICWRITE xstart[this_index] = 0;
ATOMICWRITE xend[this_index] = 0;
ATOMICWRITE xstart[this_index+1] = 0;
ATOMICWRITE xend[this_index+1] = 0;
ATOMICWRITE xstart[this_index+2] = 0;
ATOMICWRITE xend[this_index+2] = 0;
}
}
else
{
const CartesianCoordinate3D<float> p1 = lor_points.p1()*rescale;
const CartesianCoordinate3D<float> p2 = lor_points.p2()*rescale;
#ifdef STIR_OPENMP
#pragma omp atomic write
#endif
xstart[this_index] = p1[1];
#ifdef STIR_OPENMP
#pragma omp atomic write
#endif
xend[this_index] = p2[1];
#ifdef STIR_OPENMP
#pragma omp atomic write
#endif
xstart[this_index+1] = p1[2];
#ifdef STIR_OPENMP
#pragma omp atomic write
#endif
xend[this_index+1] = p2[2];
#ifdef STIR_OPENMP
#pragma omp atomic write
#endif
xstart[this_index+2] = p1[3];
#ifdef STIR_OPENMP
#pragma omp atomic write
#endif
xend[this_index+2] = p2[3];
const auto p1 = lor_points.p1()*rescale;
const auto p2 = lor_points.p2()*rescale;
CRITICALSECTION
{
ATOMICWRITE xstart[this_index] = p1[1];
ATOMICWRITE xend[this_index] = p2[1];
ATOMICWRITE xstart[this_index+1] = p1[2];
ATOMICWRITE xend[this_index+1] = p2[2];
ATOMICWRITE xstart[this_index+2] = p1[3];
ATOMICWRITE xend[this_index+2] = p2[3];
}
}
}
index += p_info.get_num_tangential_poss()*3;
Expand Down
11 changes: 10 additions & 1 deletion src/swig/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# Copyright 2012 Kris Thielemans
# Copyright 2014, 2018, 2020, 2022 University College London
# Copyright 2014, 2018, 2020, 2022, 2023 University College London
# This file is part of STIR.
#
# SPDX-License-Identifier: Apache-2.0
Expand Down Expand Up @@ -151,6 +151,9 @@ if(BUILD_SWIG_PYTHON)
SWIG_ADD_MODULE(stir python stir.i $<TARGET_OBJECTS:stir_registries>)
else()
SWIG_ADD_LIBRARY(stir LANGUAGE python TYPE MODULE SOURCES stir.i $<TARGET_OBJECTS:stir_registries>)
if("${CMAKE_CXX_COMPILER_ID}" MATCHES "MSVC")
set_property(TARGET ${SWIG_MODULE_stir_REAL_NAME} PROPERTY SWIG_GENERATED_COMPILE_OPTIONS /bigobj)
endif()
endif()
SWIG_WORKAROUND(${SWIG_MODULE_stir_REAL_NAME})
SWIG_LINK_LIBRARIES(stir ${STIR_LIBRARIES} ${STIR_Python_dependency})
Expand Down Expand Up @@ -198,6 +201,9 @@ if (BUILD_SWIG_OCTAVE)
SWIG_ADD_MODULE(stiroct octave stir.i $<TARGET_OBJECTS:stir_registries>)
else()
SWIG_ADD_LIBRARY(stiroct LANGUAGE octave TYPE MODULE SOURCES stir.i $<TARGET_OBJECTS:stir_registries>)
if("${CMAKE_CXX_COMPILER_ID}" MATCHES "MSVC")
set_property(TARGET ${SWIG_MODULE_stiroct_REAL_NAME} PROPERTY SWIG_GENERATED_COMPILE_OPTIONS /bigobj)
endif()
endif()
SET_TARGET_PROPERTIES(${SWIG_MODULE_stiroct_REAL_NAME} PROPERTIES SUFFIX ${OCTAVE_SUFFIX} PREFIX "${OCTAVE_PREFIX}")
SWIG_WORKAROUND(${SWIG_MODULE_stiroct_REAL_NAME})
Expand All @@ -224,6 +230,9 @@ if (BUILD_SWIG_MATLAB)
SWIG_ADD_MODULE(stirMATLAB matlab stir.i $<TARGET_OBJECTS:stir_registries>)
else()
SWIG_ADD_LIBRARY(stirMATLAB LANGUAGE matlab TYPE MODULE SOURCES stir.i $<TARGET_OBJECTS:stir_registries>)
if("${CMAKE_CXX_COMPILER_ID}" MATCHES "MSVC")
set_property(TARGET ${SWIG_MODULE_stirMATLAB_REAL_NAME} PROPERTY SWIG_GENERATED_COMPILE_OPTIONS /bigobj)
endif()
endif()
if (WIN32)
set (Matlab_CXXLINKER_FLAGS "/EXPORT:mexFunction")
Expand Down