Skip to content

Commit

Permalink
STYLE: Declared m_ComputePerThreadVariables as an std::vector (2 x)
Browse files Browse the repository at this point in the history 
Declared m_ComputePerThreadVariables of both as `AdvancedImageMomentsCalculator` and `ComputeDisplacementDistribution` as an `std::vector<AlignedComputePerThreadStruct>`, instead of a raw pointer to the structs. Simplified zero-initialization of the structs. Removed manual `delete[]` statements.

Based on the second commit of pull request #132 "Improved style of m_ComputePerThreadVariable data members".

Follows C++ Core Guidelines (April 10, 2022): "Avoid calling new and delete explicitly" http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rr-ptr
  • Loading branch information
@N-Dekker
N-Dekker committed May 5, 2022
1 parent ab0c8df commit fc696f8
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 103 deletions.
11 changes: 6 additions & 5 deletions Common/Transforms/itkAdvancedImageMomentsCalculator.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@

#include "itkPlatformMultiThreader.h"

#include <vector>

namespace itk
{
/** \class AdvancedImageMomentsCalculator
Expand Down Expand Up @@ -250,7 +252,7 @@ class ITK_TEMPLATE_EXPORT AdvancedImageMomentsCalculator : public Object

protected:
AdvancedImageMomentsCalculator();
~AdvancedImageMomentsCalculator() override;
~AdvancedImageMomentsCalculator() override = default;
void
PrintSelf(std::ostream & os, Indent indent) const override;

Expand Down Expand Up @@ -310,10 +312,9 @@ class ITK_TEMPLATE_EXPORT AdvancedImageMomentsCalculator : public Object

mutable MultiThreaderParameterType m_ThreaderParameters;

mutable AlignedComputePerThreadStruct * m_ComputePerThreadVariables;
mutable ThreadIdType m_ComputePerThreadVariablesSize;
bool m_UseMultiThread;
SizeValueType m_NumberOfPixelsCounted;
mutable std::vector<AlignedComputePerThreadStruct> m_ComputePerThreadVariables;
bool m_UseMultiThread;
SizeValueType m_NumberOfPixelsCounted;

SizeValueType m_NumberOfSamplesForCenteredTransformInitialization;
InputPixelType m_LowerThresholdForCenterGravity;
Expand Down
67 changes: 21 additions & 46 deletions Common/Transforms/itkAdvancedImageMomentsCalculator.hxx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,21 +51,11 @@ AdvancedImageMomentsCalculator<TImage>::AdvancedImageMomentsCalculator()
this->m_ThreaderParameters.st_Self = this;

// Multi-threading structs
this->m_ComputePerThreadVariables = nullptr;
this->m_ComputePerThreadVariablesSize = 0;
this->m_CenterOfGravityUsesLowerThreshold = false;
this->m_NumberOfSamplesForCenteredTransformInitialization = 10000;
this->m_LowerThresholdForCenterGravity = 500;
}

//----------------------------------------------------------------------
// Destructor
template <typename TImage>
AdvancedImageMomentsCalculator<TImage>::~AdvancedImageMomentsCalculator()
{
delete[] this->m_ComputePerThreadVariables;
}

/**
* ************************* InitializeThreadingParameters ************************
*/
Expand All @@ -85,24 +75,8 @@ AdvancedImageMomentsCalculator<TImage>::InitializeThreadingParameters()
*/
const ThreadIdType numberOfThreads = this->m_Threader->GetNumberOfWorkUnits();

/** Only resize the array of structs when needed. */
if (this->m_ComputePerThreadVariablesSize != numberOfThreads)
{
delete[] this->m_ComputePerThreadVariables;
this->m_ComputePerThreadVariables = new AlignedComputePerThreadStruct[numberOfThreads];
this->m_ComputePerThreadVariablesSize = numberOfThreads;
}

/** Some initialization. */
for (ThreadIdType i = 0; i < numberOfThreads; ++i)
{
this->m_ComputePerThreadVariables[i].st_M0 = NumericTraits<ScalarType>::Zero;
this->m_ComputePerThreadVariables[i].st_M1 = NumericTraits<typename VectorType::ValueType>::Zero;
this->m_ComputePerThreadVariables[i].st_M2.Fill(NumericTraits<typename MatrixType::ValueType>::ZeroValue());
this->m_ComputePerThreadVariables[i].st_Cg = NumericTraits<typename VectorType::ValueType>::Zero;
this->m_ComputePerThreadVariables[i].st_Cm.Fill(NumericTraits<typename MatrixType::ValueType>::ZeroValue());
this->m_ComputePerThreadVariables[i].st_NumberOfPixelsCounted = NumericTraits<SizeValueType>::Zero;
}
// For each thread, assign a struct of zero-initialized values.
m_ComputePerThreadVariables.assign(numberOfThreads, AlignedComputePerThreadStruct());

} // end InitializeThreadingParameters()

Expand Down Expand Up @@ -337,12 +311,14 @@ AdvancedImageMomentsCalculator<TImage>::ThreadedCompute(ThreadIdType threadId)
}
}
/** Update the thread struct once. */
this->m_ComputePerThreadVariables[threadId].st_M0 = M0;
this->m_ComputePerThreadVariables[threadId].st_M1 = M1;
this->m_ComputePerThreadVariables[threadId].st_M2 = M2;
this->m_ComputePerThreadVariables[threadId].st_Cg = Cg;
this->m_ComputePerThreadVariables[threadId].st_Cm = Cm;
this->m_ComputePerThreadVariables[threadId].st_NumberOfPixelsCounted = numberOfPixelsCounted;
AlignedComputePerThreadStruct computePerThreadStruct;
computePerThreadStruct.st_M0 = M0;
computePerThreadStruct.st_M1 = M1;
computePerThreadStruct.st_M2 = M2;
computePerThreadStruct.st_Cg = Cg;
computePerThreadStruct.st_Cm = Cm;
computePerThreadStruct.st_NumberOfPixelsCounted = numberOfPixelsCounted;
m_ComputePerThreadVariables[threadId] = computePerThreadStruct;

} // end ThreadedCompute()

Expand All @@ -354,25 +330,24 @@ template <typename TImage>
void
AdvancedImageMomentsCalculator<TImage>::AfterThreadedCompute()
{
const ThreadIdType numberOfThreads = this->m_Threader->GetNumberOfWorkUnits();
/** Accumulate thread results. */
for (ThreadIdType k = 0; k < numberOfThreads; ++k)
for (auto & computePerThreadStruct : m_ComputePerThreadVariables)
{
this->m_M0 += this->m_ComputePerThreadVariables[k].st_M0;
this->m_M0 += computePerThreadStruct.st_M0;
for (unsigned int i = 0; i < ImageDimension; ++i)
{
this->m_M1[i] += this->m_ComputePerThreadVariables[k].st_M1[i];
this->m_Cg[i] += this->m_ComputePerThreadVariables[k].st_Cg[i];
this->m_ComputePerThreadVariables[k].st_M1[i] = 0;
this->m_ComputePerThreadVariables[k].st_Cg[i] = 0;
this->m_M1[i] += computePerThreadStruct.st_M1[i];
this->m_Cg[i] += computePerThreadStruct.st_Cg[i];
computePerThreadStruct.st_M1[i] = 0;
computePerThreadStruct.st_Cg[i] = 0;
for (unsigned int j = 0; j < ImageDimension; ++j)
{
this->m_M2[i][j] += this->m_ComputePerThreadVariables[k].st_M2[i][j];
this->m_Cm[i][j] += this->m_ComputePerThreadVariables[k].st_Cm[i][j];
this->m_ComputePerThreadVariables[k].st_M2[i][j] = 0;
this->m_ComputePerThreadVariables[k].st_Cm[i][j] = 0;
this->m_M2[i][j] += computePerThreadStruct.st_M2[i][j];
this->m_Cm[i][j] += computePerThreadStruct.st_Cm[i][j];
computePerThreadStruct.st_M2[i][j] = 0;
computePerThreadStruct.st_Cm[i][j] = 0;
}
this->m_ComputePerThreadVariables[k].st_M0 = 0;
computePerThreadStruct.st_M0 = 0;
}
}
DoPostProcessing();
Expand Down
7 changes: 4 additions & 3 deletions Common/itkComputeDisplacementDistribution.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@
#include "itkImageFullSampler.h"
#include "itkPlatformMultiThreader.h"

#include <vector>

namespace itk
{
/**\class ComputeDisplacementDistribution
Expand Down Expand Up @@ -132,7 +134,7 @@ class ITK_TEMPLATE_EXPORT ComputeDisplacementDistribution : public ScaledSingleV

protected:
ComputeDisplacementDistribution();
~ComputeDisplacementDistribution() override;
~ComputeDisplacementDistribution() override = default;

/** Typedefs for multi-threading. */
using ThreaderType = itk::PlatformMultiThreader;
Expand Down Expand Up @@ -214,8 +216,7 @@ class ITK_TEMPLATE_EXPORT ComputeDisplacementDistribution : public ScaledSingleV
private:
mutable MultiThreaderParameterType m_ThreaderParameters;

mutable AlignedComputePerThreadStruct * m_ComputePerThreadVariables;
mutable ThreadIdType m_ComputePerThreadVariablesSize;
mutable std::vector<AlignedComputePerThreadStruct> m_ComputePerThreadVariables;

SizeValueType m_NumberOfPixelsCounted;
bool m_UseMultiThread;
Expand Down
64 changes: 15 additions & 49 deletions Common/itkComputeDisplacementDistribution.hxx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,24 +56,9 @@ ComputeDisplacementDistribution<TFixedImage, TTransform>::ComputeDisplacementDis
/** Initialize the m_ThreaderParameters. */
this->m_ThreaderParameters.st_Self = this;

// Multi-threading structs
this->m_ComputePerThreadVariables = nullptr;
this->m_ComputePerThreadVariablesSize = 0;

} // end Constructor


/**
* ************************* Destructor ************************
*/

template <class TFixedImage, class TTransform>
ComputeDisplacementDistribution<TFixedImage, TTransform>::~ComputeDisplacementDistribution()
{
delete[] this->m_ComputePerThreadVariables;
} // end Destructor


/**
* ************************* InitializeThreadingParameters ************************
*/
Expand All @@ -93,22 +78,8 @@ ComputeDisplacementDistribution<TFixedImage, TTransform>::InitializeThreadingPar
*/
const ThreadIdType numberOfThreads = this->m_Threader->GetNumberOfWorkUnits();

/** Only resize the array of structs when needed. */
if (this->m_ComputePerThreadVariablesSize != numberOfThreads)
{
delete[] this->m_ComputePerThreadVariables;
this->m_ComputePerThreadVariables = new AlignedComputePerThreadStruct[numberOfThreads];
this->m_ComputePerThreadVariablesSize = numberOfThreads;
}

/** Some initialization. */
for (ThreadIdType i = 0; i < numberOfThreads; ++i)
{
this->m_ComputePerThreadVariables[i].st_MaxJJ = 0.0;
this->m_ComputePerThreadVariables[i].st_Displacement = 0.0;
this->m_ComputePerThreadVariables[i].st_DisplacementSquared = 0.0;
this->m_ComputePerThreadVariables[i].st_NumberOfPixelsCounted = NumericTraits<SizeValueType>::Zero;
}
// For each thread, assign a struct of zero-initialized values.
m_ComputePerThreadVariables.assign(numberOfThreads, AlignedComputePerThreadStruct());

} // end InitializeThreadingParameters()

Expand Down Expand Up @@ -450,11 +421,12 @@ ComputeDisplacementDistribution<TFixedImage, TTransform>::ThreadedCompute(Thread
}

/** Update the thread struct once. */
this->m_ComputePerThreadVariables[threadId].st_MaxJJ = maxJJ;
this->m_ComputePerThreadVariables[threadId].st_Displacement = displacement;
this->m_ComputePerThreadVariables[threadId].st_DisplacementSquared = displacementSquared;
this->m_ComputePerThreadVariables[threadId].st_NumberOfPixelsCounted = numberOfPixelsCounted;

AlignedComputePerThreadStruct computePerThreadStruct;
computePerThreadStruct.st_MaxJJ = maxJJ;
computePerThreadStruct.st_Displacement = displacement;
computePerThreadStruct.st_DisplacementSquared = displacementSquared;
computePerThreadStruct.st_NumberOfPixelsCounted = numberOfPixelsCounted;
m_ComputePerThreadVariables[threadId] = computePerThreadStruct;
} // end ThreadedCompute()


Expand All @@ -466,28 +438,22 @@ template <class TFixedImage, class TTransform>
void
ComputeDisplacementDistribution<TFixedImage, TTransform>::AfterThreadedCompute(double & jacg, double & maxJJ)
{
const ThreadIdType numberOfThreads = this->m_Threader->GetNumberOfWorkUnits();

/** Reset all variables. */
maxJJ = 0.0;
double displacement = 0.0;
double displacementSquared = 0.0;
this->m_NumberOfPixelsCounted = 0.0;

/** Accumulate thread results. */
for (ThreadIdType i = 0; i < numberOfThreads; ++i)
for (const auto & computePerThreadStruct : m_ComputePerThreadVariables)
{
maxJJ = std::max(maxJJ, this->m_ComputePerThreadVariables[i].st_MaxJJ);
displacement += this->m_ComputePerThreadVariables[i].st_Displacement;
displacementSquared += this->m_ComputePerThreadVariables[i].st_DisplacementSquared;
this->m_NumberOfPixelsCounted += this->m_ComputePerThreadVariables[i].st_NumberOfPixelsCounted;

/** Reset all variables for the next resolution. */
this->m_ComputePerThreadVariables[i].st_MaxJJ = 0;
this->m_ComputePerThreadVariables[i].st_Displacement = 0;
this->m_ComputePerThreadVariables[i].st_DisplacementSquared = 0;
this->m_ComputePerThreadVariables[i].st_NumberOfPixelsCounted = 0;
maxJJ = std::max(maxJJ, computePerThreadStruct.st_MaxJJ);
displacement += computePerThreadStruct.st_Displacement;
displacementSquared += computePerThreadStruct.st_DisplacementSquared;
this->m_NumberOfPixelsCounted += computePerThreadStruct.st_NumberOfPixelsCounted;
}
// Reset all variables for the next resolution.
std::fill_n(m_ComputePerThreadVariables.begin(), m_ComputePerThreadVariables.size(), AlignedComputePerThreadStruct());

/** Compute the sigma of the distribution of the displacements. */
const double meanDisplacement = displacement / this->m_NumberOfPixelsCounted;
Expand Down

0 comments on commit fc696f8

Please sign in to comment.