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

update Array hierarchy and allocate nD arrays in a contiguous block by default #1236

Merged
merged 25 commits into from
May 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
4ca1217
added size_all() to IndexRange
KrisThielemans Mar 31, 2023
1873c9d
allow Vector/Array construction from existing memory
KrisThielemans Mar 31, 2023
3e9527b
add some timings to test_Array (to be disabled later)
KrisThielemans Apr 18, 2023
77cbf5a
speed-up IndexRange::size_all for regular ranges
KrisThielemans Aug 20, 2023
ae72749
Array: first version that defaults to continuous allocation
KrisThielemans Aug 21, 2023
616dbb5
Array: add move constructors, swap and missing assignment operators
KrisThielemans Aug 23, 2023
49146ce
Array: clean-up
KrisThielemans Aug 24, 2023
86a449c
Array: remove wrong/not implemented constructors
KrisThielemans Aug 24, 2023
205197c
resolve std::swap for VC
KrisThielemans Aug 24, 2023
bc5a282
add Array constructor from ptr [ci skip]
KrisThielemans Feb 4, 2024
a399a96
Array: first version using shared_ptr<T[]> (WIP)
KrisThielemans Feb 11, 2024
12cec9a
require C++-17
KrisThielemans Feb 11, 2024
3c29a2a
Array: use shared_ptr<T[]> for VectorWithOffset
KrisThielemans Feb 11, 2024
f4b4db8
[SWIG] ignore swap and *full_data_ptr
KrisThielemans Feb 12, 2024
2ba9924
Array: remove private member _owns_memory_for_data
KrisThielemans Feb 12, 2024
433bd82
Array: clean-up constructors that take existing data
KrisThielemans Feb 12, 2024
0ae05b1
run clang-format
KrisThielemans Feb 15, 2024
345b0d7
remove experimental/memory work-around for shared_ptr
KrisThielemans Feb 16, 2024
be32661
remove VS 2015 job as no C++-17 for shared_ptr<float[]>
KrisThielemans Feb 16, 2024
510e429
[CMake] fix FindCERN_ROOT to always check version
KrisThielemans Feb 17, 2024
715ede8
[CMake] require ROOT 6.28.0 (needed for C++-17)
KrisThielemans Feb 17, 2024
4bf2d13
optimise reading/writing for contiguous arrays
KrisThielemans May 17, 2024
74579af
[SWIG] avoid warning on stir::swap
KrisThielemans May 17, 2024
8752e45
Avoid image copies in Parallelproj
KrisThielemans May 17, 2024
ae82826
created release_6.2.htm with info on Array PR
KrisThielemans May 23, 2024
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
1 change: 0 additions & 1 deletion .appveyor.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ version: '{build}'
os:
- Visual Studio 2022
- Visual Studio 2019
- Visual Studio 2015

platform:
- x64
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/build-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ jobs:
compiler: gcc
compiler_version: 12
cuda_version: "12.1.0"
BUILD_FLAGS: "-DSTIR_OPENMP=ON -DCMAKE_CXX_STANDARD=14"
BUILD_FLAGS: "-DSTIR_OPENMP=ON -DCMAKE_CXX_STANDARD=17"
BUILD_TYPE: "Release"
parallelproj: "ON"
ROOT: "OFF"
Expand Down
10 changes: 2 additions & 8 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ include(src/cmake/SetC++Version.cmake)

# minimum C++ version required (you can still ask for a more recent one
# by setting CMAKE_CXX_STANDARD)
UseCXX(14)
UseCXX(17)

# set default build-type to Release
if(NOT CMAKE_BUILD_TYPE)
Expand Down Expand Up @@ -118,18 +118,12 @@ if(NOT DISABLE_LLN_MATRIX)
endif()

if(NOT DISABLE_CERN_ROOT)
find_package(CERN_ROOT)
find_package(CERN_ROOT 6.28.00) # required for C++17
if (CERN_ROOT_FOUND)
message(STATUS "ROOT Version: ${CERN_ROOT_VERSION}")
if (${CERN_ROOT_VERSION} VERSION_GREATER 6.29.99)
message(STATUS "ROOT Version is >= 6.30. Setting the minimum CXX version to 20.")
UseCXX(20)
elseif (${CERN_ROOT_VERSION} VERSION_GREATER 6.27.99)
message(STATUS "ROOT Version is >= 6.28. Setting the minimum CXX version to 17.")
UseCXX(17)
elseif (${CERN_ROOT_VERSION} VERSION_GREATER 6.23.99)
message(STATUS "ROOT Version is >= 6.24. Setting the minimum CXX version to 14.")
UseCXX(14)
endif()
endif()
endif()
Expand Down
125 changes: 125 additions & 0 deletions documentation/release_6.2.htm
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
<!DOCTYPE HTML>
<html lang="en">
<head>
<title>Summary of changes in STIR release 6.2</title>
</head>

<body>
<h1>Summary of changes in STIR release 6.2</h1>

<p>
This version is 100% backwards compatible with STIR 6.1. However, C++-17 is now required.
</p>

<h2>Overall summary</h2>
<p>

</p>

<p>
Of course, there is also the usual code-cleanup and improvements to the documentation.
</p>

<p>
This release contains mainly code written by Nicole Jurjew (UCL) and Kris Thielemans (UCL).
</p>

<h2>Patch release info</h2>
<ul>
<li>
6.2.0 released ??/2024<br>
<a href="https://github.com/UCL/STIR/milestone/11">GitHub Milestone 6.2</a>
</li>
<!--
<li> 4.0.1 released 28/04/2020
<ul>
<li><a href=https://github.com/UCL/STIR/pull/513>PR 513</a> (suppress warnings with clang)</li>
</ul>
-->
</ul>

<h2> Summary for end users (also to be read by developers)</h2>


<h3>New functionality</h3>
<ul>
</ul>


<h3>Changed functionality</h3>
<ul>
</ul>


<h3>Bug fixes</h3>
<ul>
</ul>

<h3>Build system</h3>
<ul
<li>
C++-17 is now required.
</li>
</ul>

<h3>Known problems</h3>
<p>See <a href=https://github.com/UCL/STIR/labels/bug>our issue tracker</a>.</p>


<H2>What's new for developers (aside from what should be obvious
from the above):</H2>

<h3>Changed functionality</h3>
<ul>
<li>
<code>Array</code> classes by default use contiguous memory allocation (as opposed to a sequence of 1D vectors).
This could speed up memory allocation and destruction of arrays with a high number of underlying 1D vectors. It also allows reading/writing
data in one call to the C++ library, as opposed to many small calls. Also added move constructors to the <code>Array</code>,
<code>VectorWithOffset</code> classes.
<br>
<a href=https://github.com/UCL/STIR/pull/1236>issue #1236</a>.
</li>
</ul>

<h3>Bug fixes</h3>
<ul>
<li>
<code>PoissonLogLikelihoodWithLinearModelForMeanAndProjData</code> had a (minor?) problem with TOF data
that when computing the gradient, the normalisation object was not set-up with the TOF data,
but non-TOF instead. This did not happen in our normal reconstruction code, and would have thrown an error
if it occured.
<br>
Fixed in <a href=https://github.com/UCL/STIR/pull/1427>issue #1427</a>.
</li>
</ul>


<h3>Other code changes</h3>
<ul>
<li>
Fixes an incompatibility with C++20.
</li>
</ul>

<h3>Build system</h3>
<ul>
<li>
Force C++ version according to CERN ROOT versions: ROOT 6.28.10 needs C++17 and 6.30.2 needs C++20.
Also some fixes when relying on <code>root-config</code>.
</li>
</ul>

<h3>Test changes</h3>

<h4>C++ tests</h4>
<ul>
<li>
Objective functions (both projection-data and list-mode) and priors now have a numerical test for <code>accumulate_Hessian_times_input</code>
<br>
<a href=https://github.com/UCL/STIR/pull/1418> PR #1418</a>
</li>
</ul>

</body>

</html>
3 changes: 1 addition & 2 deletions src/buildblock/ProjDataInfoCylindrical.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ using std::min_element;
using std::max_element;
using std::min;
using std::max;
using std::swap;
using std::endl;
using std::string;
using std::pair;
Expand Down Expand Up @@ -113,7 +112,7 @@ ProjDataInfoCylindrical::ProjDataInfoCylindrical(const shared_ptr<Scanner>& scan
min_ring_diff[segment_num],
max_ring_diff[segment_num],
segment_num);
swap(min_ring_diff[segment_num], max_ring_diff[segment_num]);
std::swap(min_ring_diff[segment_num], max_ring_diff[segment_num]);
}
}

Expand Down
4 changes: 3 additions & 1 deletion src/cmake/FindCERN_ROOT.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -168,4 +168,6 @@ if (CERN_ROOT_DEBUG)
endif()

INCLUDE(FindPackageHandleStandardArgs)
FIND_PACKAGE_HANDLE_STANDARD_ARGS(CERN_ROOT "CERN ROOT not found. If you do have it, set ROOT_DIR (preferred), ROOTSYS or add root-config to your path" CERN_ROOT_VERSION CERN_ROOT_LIBRARIES CERN_ROOT_INCLUDE_DIRS)
FIND_PACKAGE_HANDLE_STANDARD_ARGS(CERN_ROOT FAIL_MESSAGE "CERN ROOT not found. If you do have it, set ROOT_DIR (preferred), ROOTSYS or add root-config to your path"
VERSION_VAR CERN_ROOT_VERSION
REQUIRED_VARS CERN_ROOT_LIBRARIES CERN_ROOT_INCLUDE_DIRS)
Loading
Loading