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

• No API changes were made (or the changes have been approved)
• No major design changes were made (or the changes have been approved)
• Added test (or behavior not changed)
• Updated API documentation (or API not changed)
• Added license to new files (if any)
• Added Python wrapping to new files (if any) as described in ITK Software Guide Section 9.5
• 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.

[dzenanz]

In the commit message, label map filers -> label map filters. The rest looks good on a glance.

[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.

[thewtex]

Great to have these addressed! 🥇

[seanm]

This solves the itkStatisticsUniqueLabelMapFilterTest1 test failure under TSan! Amazing! We'll see on cdash if it solves other similar failures too...

[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