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

ENH: BinaryMathematicalMorphology baseline testing #219

Conversation

mseng10
Copy link
Contributor

@mseng10 mseng10 commented Mar 9, 2021

This enables baseline testing for the BinaryMathematicalMorphology
module. This specifically does the following:

  • Defines OutputBaseline for all baseline tests in Filtering/BinaryMathematicalMorphology/CMakeLists.txt
  • Implements WriteImage function for ClosingBinaryImage, OpenBinaryImage and PruneBinaryImage. This that takes an Image and output filename as arguments and saves the output image to the output image filename.
  • Saves new baseline images for ClosingBinaryImage, OpenBinaryImage, and
    PruneBinaryIamge.
  • Changes original output ClosingBinaryImage.png, OpenBinaryImage.png and
    PruneBinaryImage.png to clarify these are outputs in the QuickView
    window.
  • Allow user to define input and output file paths for
    ClosingBinaryImage, OpenBinaryImage, PruneBinaryImage.

@mseng10 mseng10 force-pushed the binary-mathematical-morphology-baseline branch from 629e353 to de347bf Compare March 11, 2021 15:49
@mseng10 mseng10 force-pushed the binary-mathematical-morphology-baseline branch from de347bf to d57fa4a Compare March 16, 2021 17:23
@thewtex
Copy link
Member

thewtex commented Mar 17, 2021

@mseng10 can this please be rebased on master for updated CI?

@mseng10 mseng10 force-pushed the binary-mathematical-morphology-baseline branch from d57fa4a to c944bb3 Compare March 18, 2021 14:45
@mseng10
Copy link
Contributor Author

mseng10 commented Mar 23, 2021

@dzenanz @thewtex It seems the tests are failing on the Windows builds as the baseline images differ from the test's output images. Is there any reason why this would be working on the linux and macOS builds, but not Windows?

@dzenanz
Copy link
Member

dzenanz commented Mar 23, 2021

Looking at https://open.cdash.org/test/366062497, the differences are stark. It does not seem to work properly on Windows.

@mseng10
Copy link
Contributor Author

mseng10 commented Mar 23, 2021

Looking at https://open.cdash.org/test/366062497, the differences are stark. It does not seem to work properly on Windows.

I see. I believe this may be a symptom of a bigger issue as I have started #212 and this is producing some similar results in the Windows builds (https://open.cdash.org/viewTest.php?onlyfailed&buildid=7117682). They work fine in Linux and MacOS. I will take a deeper dive into this to try and diagnose the issue.

@thewtex
Copy link
Member

thewtex commented May 18, 2021

Rebasing on master.

@thewtex thewtex force-pushed the binary-mathematical-morphology-baseline branch from c944bb3 to e739d4f Compare May 18, 2021 14:19
@jhlegarreta jhlegarreta force-pushed the binary-mathematical-morphology-baseline branch from e739d4f to b625917 Compare September 11, 2021 17:59
@jhlegarreta
Copy link
Member

b625917 rebased on master to solve conflicts.

@jhlegarreta jhlegarreta force-pushed the binary-mathematical-morphology-baseline branch 2 times, most recently from b051505 to 8e28b03 Compare September 11, 2021 19:43
@jhlegarreta
Copy link
Member

jhlegarreta commented Sep 11, 2021

Either the baselines seem to need an update:
https://open.cdash.org/viewTest.php?onlyfailed&buildid=7451836
e.g.
https://open.cdash.org/test/487486679

Or the test images do not look good. Anyone having the time to update the necessary images?

@dzenanz
Copy link
Member

dzenanz commented Sep 16, 2021

This bug seems to affect many more examples. A search for image->Allocate(); has 149 hits while image->FillBuffer( has 64 hits.

@jhlegarreta jhlegarreta force-pushed the binary-mathematical-morphology-baseline branch from 6adfec0 to f785ae9 Compare September 17, 2021 00:48
mseng10 and others added 2 commits September 16, 2021 21:57
This enables baseline testing for the BinaryMathematicalMorphology
module. This specifically does the following:

- Defines OutputBaseline for all baseline tests in Filtering/BinaryMathematicalMorphology/CMakeLists.txt
- Implements ITK's WriteImage function for ClosingBinaryImage, OpenBinaryImage and PruneBinaryImage.
- Saves new baseline images for ClosingBinaryImage, OpenBinaryImage, and
  PruneBinaryIamge.
- Changes original output ClosingBinaryImage.png, OpenBinaryImage.png and
  PruneBinaryImage.png to clarify these are outputs in the QuickView
window.
- Allow user to define input and output file paths for
  ClosingBinaryImage, OpenBinaryImage, PruneBinaryImage.
@jhlegarreta jhlegarreta force-pushed the binary-mathematical-morphology-baseline branch from f785ae9 to cbc92e9 Compare September 17, 2021 02:10
@jhlegarreta
Copy link
Member

jhlegarreta commented Sep 17, 2021

Not sure why the doc build was cancelled. It seems like we cannot run just that one. The rest are passing.

Edit: I'd say that the netlify deploy seems that the docs/site were correctly generated:
https://mseng-itkexamples-e15fb6.netlify.app/src/filtering/binarymathematicalmorphology/

However, it also shows that:

  • Closing Binary Image and Opening Binary Image provide the (seemingly) same output, which I'd say is not correct.
  • Prune Binary Image seems not to have an effect on the output.

Not sure if best would be to leave them for a separate issue/PR.

@dzenanz
Copy link
Member

dzenanz commented Sep 17, 2021

build-test-documentation (ubuntu-18.04):
System.TimeoutException: The HTTP request timed out after 00:01:40.

@dzenanz dzenanz merged commit fc491a4 into InsightSoftwareConsortium:master Sep 17, 2021
@dzenanz
Copy link
Member

dzenanz commented Sep 17, 2021

The input images are not the same. Pruning has an "erroneous" pixel.

You are right that closing should produce rounded corners.

@dzenanz
Copy link
Member

dzenanz commented Sep 17, 2021

I created #308 to keep track of the allocate issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants