Fix Unique LabelMap filters producing non-unique labels and causing concurrent pixel writes.

[blowekamp]

Address bugs in unique label map filters Two bugs were addressed which could cause overlapping and/or errouneous outputs.

1. When a line we trimmed in the front, it is not placed back into the queue to ensure that it is still the first one, and the asserted assumptions of the order remain true.
2. Fix comparisons between start and ends to be correct for an interval of pixels where the start+length is not included.

closes #4655 closes #3031

PR Checklist

• [x] No API changes were made (or the changes have been approved)
• [x] No major design changes were made (or the changes have been approved)
• [x] Added test (or behavior not changed)
• [x] Updated API documentation (or API not changed)
• [x] Added license to new files (if any)
• [x] Added Python wrapping to new files (if any) as described in ITK Software Guide Section 9.5
• [x] Added ITK examples for all new major features (if any)

Refer to the ITK Software Guide for further development details if necessary.

[seanm]

@blowekamp are you set up to use TSan, or do you want me to try with patch with TSan?

[dzenanz]

I believe this does not patch it, it just explicitly checks for label overlaps.

[blowekamp]

I think this is pretty close to fixing the issue, more testing is needed.

[blowekamp]

@seanm You can run the sanitizer. However I was able to detect the bug with additional checks before the threading issue occurs.

[seanm]

I'm getting a build error:

/Users/sean/external/ITK/Modules/Filtering/LabelMap/test/itkStatisticsUniqueLabelMapFilterTest1.cxx:46:7: error: use of undeclared identifier 'TEST_EXPECT_TRUE'
/Users/sean/external/ITK/Modules/Filtering/LabelMap/test/itkStatisticsUniqueLabelMapFilterTest1.cxx:158:18: note: in instantiation of function template specialization 'CheckLabelMapOverlap<itk::LabelMap<itk::StatisticsLabelObject<unsigned char, 2>>>' requested here
  int exitCode = CheckLabelMapOverlap(unique->GetOutput());
                 ^

and

/Users/sean/external/ITK/Modules/Filtering/LabelMap/test/itkStatisticsUniqueLabelMapFilterTest1.cxx:46:7: error: attempt to use a poisoned identifier
      TEST_EXPECT_TRUE(line.GetLength() <= labelObject->GetNumberOfPixels());
      ^

[blowekamp]

@seanm Thanks looks like that should have been ITK_TEST_EXPECT_TRUE. I am surprised that did not produce a warning on any of the builds.

[seanm]

So this is mergeable now?

[blowekamp]

@thewtex I was hoping to get this in for 5.4 but missed it. The current target branch is release-5.4. What should be do to merge this patch?

[thewtex]

@thewtex I was hoping to get this in for 5.4 but missed it. The current target branch is release-5.4. What should be do to merge this patch?

@blowekamp outstanding!

I will go ahead and merge this one. For future reference, the process is:

1. merge topic branch into release-5.4 via GitHub Web UI
2. merge release-5.4 into release locally (git checkout release-5.4; git pull --ff-only upstream release-5.4; git checkout release; git pull --ff-only upstream release; git merge release-5.4)
3. merge release into master locally (git checkout master; git pull --ff-only upstream master; git merge release)
4. locally git push upstream release master

[seanm]

Cool, I'm excited to see on tomorrow's TSan cdash is this fixes any other tests too:

https://open.cdash.org/builds/9649990

[seanm]

Seems it fixed itkStatisticsUniqueLabelMapFilterTest2, but not any other test.

[blowekamp]

Seems it fixed itkStatisticsUniqueLabelMapFilterTest2, but not any other test.

Are any of the other failure related to label maps?

[seanm]

Are any of the other failure related to label maps?

I haven't analyzed each in depth, but here's the list:

https://open.cdash.org/viewTest.php?onlyfailed&buildid=9652708