BUG: Fix test errors steming from the new multi-threading mechanism.

[jhlegarreta]

Fix errors steming from the use of the new itk::PoolMultiThreader class for multi-threading.

The solution adopted in this patch set was based on the discussion in: http://review.source.kitware.com/#/c/23434/

[phcerdan]

Nice, have you tested it? Not sure if the continuous integration is really working.

[jhlegarreta]

@phcerdan Just built it locally with MSVC 2015. The build was fine. But I did not check the tests. The PR to ITKTextureFeatures is also reporting errors (although different), so it seems to me that we're still missing something.

[phcerdan]

I am hitting errors running tests, even with this PR, using FFTW. It seems the rename from ThreadedGenerateData, to Dynamic... has not been performed in that module.

So, I understand every external module and third parties have to rename ThreadedGeneratedData? Would it be possible to use a macro, or a general cmake option for the user to choose behavior at configure time?

[phcerdan]

Ok, nevermind, I see a patch for the FFTW module: http://review.source.kitware.com/#/c/23443/

Also, related review about threaded mechanisms: http://review.source.kitware.com/#/c/23439/

[jhlegarreta]

@phcerdan You're right; this is related to test failures steming from http://review.source.kitware.com/#/c/23175/

And a solution is being sought in: http://review.source.kitware.com/#/c/23439/

Builds were fine, but tests got broken.

Once the appropriate solution is found, I'll apply it to all remotes, and will update this branch/PR. Also, Matt will update the ITK version for the CI builds :ok_hand:

[thewtex]

The build should work after updating the CI to use the configuration provided by ITKModuleTemplate.

[phcerdan]

@thewtex @jhlegarreta I merged latest ITKModuleTemplate, and I think the CI works now!

Is there a way to trigger CI manually in github? Would it use the latest master?

[jhlegarreta]

@phcerdan awesome. If you do not have a Restart build button available at the top, right corner (just below More options) you may need to enable ITKIsotropicWavelets in your profile settings?

As for whether it would use the latest master, I'm not sure how travis fetches the ITK version; @thewtex will be able to answer this.

[thewtex]

The build-and-test CircleCI build is based off the insighttoolkit/module-ci:latest image defined here:

https://github.com/InsightSoftwareConsortium/ITKContinuousIntegration/blob/master/BuildRobot/module-ci/Dockerfile

This was just updated to ITK master as of today.

[phcerdan]

@jhlegarreta Found the restart build button! Thanks :)

[phcerdan]

Hitting a few errors.

The only logical one I face is this, and maybe @dzenanz has an idea of the best design to overcome this. I was using a barrier to calculate some image stats only once. But without threadID, what would be the way to do this?

template< typename TInputImage, typename TOutputImage >
void
PhaseAnalysisSoftThresholdImageFilter< TInputImage, TOutputImage >
::DynamicThreadedGenerateData( const OutputImageRegionType & outputRegionForThread )
{
  Superclass::DynamicThreadedGenerateData(outputRegionForThread);

  auto phasePtr     = this->GetOutputPhase();
  auto amplitudePtr = this->GetOutputAmplitude();
  auto outputPtr    = this->GetOutputCosPhase();

  // Wait for mean/variance calculation. Stats only once.
  if ( this->GetApplySoftThreshold() )
    {
    m_Barrier1->Wait();
    if ( threadId == this->GetNumberOfThreads() - 1 ) // <<<< HERE
      {
      using StatisticsImageFilter = itk::StatisticsImageFilter< OutputImageType >;
      auto statsFilter = StatisticsImageFilter::New();
      statsFilter->SetInput(amplitudePtr);
      statsFilter->Update();
      this->m_MeanAmp   = statsFilter->GetMean();
      this->m_SigmaAmp  = sqrt(statsFilter->GetVariance());
      this->m_Threshold = this->m_MeanAmp + this->m_NumOfSigmas * this->m_SigmaAmp;
      }
    m_Barrier2->Wait();
    }
/// Keep going with the filter using all the threads available in parallel.

[dzenanz]

Pattern with Barrier:

ThreadedGenerateData()
{
//code1 (parallel)
myBarrier->Wait();
if (threadId==0)
  //code2 single-threaded
//code3 (parallel)
}

after refactoring to not use barrier:

GenerateData() //Not Threaded
{
this->AllocateOutputs();
this->BeforeThreadedGenerateData();
ParallelizeImageRegion(code1 as lambda)
//if code1 is more than a few lines of code, 
//make it a separate method which the lambda invokes
//code2 single-threaded
ParallelizeImageRegion(code3 as lambda)
this->AfterThreadedGenerateData();
}

[dzenanz]

There are different examples of similar refactoring in this commit.

[phcerdan]

So powerful, thanks @dzenanz, I am going to give it a try to remove the barrier.

[phcerdan]

Bum! I think it works. Great stuff @dzenanz . Could you please have a quick look to #81 ? I used lambdas instead of binding.

[phcerdan]

Closing because follow up on #81