Skip to content

Test #2557: itkOptImageToImageMetricsTest04 fails on single-CPU systems #5317

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

Closed
sanvila opened this issue Apr 21, 2025 · 6 comments · Fixed by #5354
Closed

Test #2557: itkOptImageToImageMetricsTest04 fails on single-CPU systems #5317

sanvila opened this issue Apr 21, 2025 · 6 comments · Fixed by #5354
Labels
type:Bug Inconsistencies or issues which will cause an incorrect result under some or all circumstances

Comments

@sanvila
Copy link

sanvila commented Apr 21, 2025

Description

Every time I try to build (the Debian package of) insighttoolkit on a single-CPU system,
the test in the title fails:

test 2557
          Start 2557: itkOptImageToImageMetricsTest04

2557: Test command: /<<PKGBUILDDIR>>/BUILD/bin/ITKReviewTestDriver "itkOptImageToImageMetricsTest2" "/<<PKGBUILDDIR>>/Examples/Data/DiagonalLines.png" "/<<PKGBUILDDIR>>/BUILD/ExternalData/Modules/Nonunit/Review/test/Baseline/itkBSplineTransformTest4.png"
2557: Working Directory: /<<PKGBUILDDIR>>/BUILD/Modules/Nonunit/Review/test
2557: Test timeout computed to be: 1500
2557: OPTIMIZED ON
2557: Default number of threads : 1
2557: ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
2557: Now Running tests with : 
2557:    Global Default Number of Threads 1
2557:    Global Maximum Number of Threads 128
2557: ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
2557: 
2557: libpng warning: sCAL: invalid unit
2557: libpng warning: sCAL: invalid unit
2557: libpng warning: sCAL: invalid unit
2557: libpng warning: sCAL: invalid unit
2557: -------------------------------------------------------------------
2557: Testing
2557:   Metric       : MeanSquaresImageToImageMetric
2557:   Interpolator : LinearInterpolateImageFunction
2557:   Transform    : BSplineTransform
2557: -------------------------------------------------------------------
2557: 
2558/3183 Test #2557: itkOptImageToImageMetricsTest04 ........................................................................***Failed    0.05 sec
OPTIMIZED ON
Default number of threads : 1
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

This is very strange because there are more than 2500 tests and only one of them fails.

Steps to Reproduce

Build the package on a single-CPU system, including the tests.

Alternatively (if there is not a single-cpu VM available) try booting with GRUB_CMDLINE_LINUX="nr_cpus=1"

Expected behavior

The expected behavior is that all tests pass.

Actual behavior

The actual behavior is that the tests in the title fails.

Reproducibility

The problem happens always when the tests are executed on a single-cpu system.

Versions

This has been happening at least since version 4.13.3.

The last version I tried (from the Debian package) is 5.4.3 and it still happens.

Environment

I'm using Debian unstable, but I don't think that's relevant, as the failure is very consistent regardless of everything else.

Additional Information

I'd like to know if this is a bug in the code, a bug in the test, or maybe I am expected to skip the test when building the package on a single-cpu system. For the Debian package, I already have a fix ready, involving debian/rules, the script controlling the build:

CTEST_ARGS :=
ifeq ($(shell nproc), 1)
  CTEST_ARGS += -E itkOptImageToImageMetricsTest04
endif

but of course it would be a lot better if now that the problem is known this could be done by the test itself.

Thanks.

@sanvila sanvila added the type:Bug Inconsistencies or issues which will cause an incorrect result under some or all circumstances label Apr 21, 2025
Copy link

Thank you for contributing an issue! 🙏

Welcome to the ITK community! 🤗👋☀️

We are glad you are here and appreciate your contribution. Please keep in mind our community participation guidelines. 📜
Also, please check existing open issues and consider discussion on the ITK Discourse. 📖

This is an automatic message. Allow for time for the ITK community to be able to read the issue and comment on it.

@dzenanz
Copy link
Member

dzenanz commented Apr 21, 2025

@N-Dekker can you look into this?

@N-Dekker
Copy link
Contributor

N-Dekker commented Apr 22, 2025

@sanvila Hi Santiago Vila! Thank you for reporting this issue! Can you please tell us if the problem still appears when you locally remove the following five lines of code from "itkOptImageToImageMetricsTest2.h"?

const TransformPointer * transformPtr = metric->GetThreaderTransform();
if ((transformPtr == static_cast<const TransformPointer *>(nullptr)) || (transformPtr[0].IsNull()))
{
exit(EXIT_FAILURE);
}


For the record, these lines of code appear introduced, originally, with commit 9ba4527, March 5, 2010:

const TransformPointer *transformPtr= metric->GetThreaderTransform();
if ((transformPtr==static_cast<const TransformPointer *>(NULL))||
(transformPtr[0].IsNull()))
{
exit(EXIT_FAILURE);
}

@sanvila
Copy link
Author

sanvila commented Apr 22, 2025

Can you please tell us if the problem still appears when you locally remove the following five lines of code from "itkOptImageToImageMetricsTest2.h"?

I confirm that removing those lines fixes the problem!
Will that be the official fix?

Thanks a lot!

(p.s. In case you wonder: I can build 99.9% of all Debian packages using a single CPU, but Debian does not have a control field to tell the user "this package requires more than one CPU to be built", so I'm trying to close the gap).

@N-Dekker
Copy link
Contributor

N-Dekker commented Apr 22, 2025

You're welcome, @sanvila

I confirm that removing those lines fixes the problem! Will that be the official fix?

It's not yet official, but it's possible 😃 Another possibility could (possibly) be:

if (transformPtr != nullptr && transformPtr[0].IsNull())
{ 
   exit(EXIT_FAILURE); 
} 

Or something else, in pseudo-code:

if (number of threads is greater than one) { do the original check }

Is the original check still important to anyone? Otherwise it's OK to me to remove it. Please feel free to make it a pull request.

@sanvila
Copy link
Author

sanvila commented May 16, 2025

Please feel free to make it a pull request.

Hi. Sorry for the late reply. Not sure if that was for me or for the other ITK authors. In case a pull request is all you need, here it is. I've put you as the commit author, since you are the one who found the fix and I'm just the bug reporter here (no need for credit, really). Feel free to change anything to adapt to any style guide which I may have missed.

Thanks a lot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:Bug Inconsistencies or issues which will cause an incorrect result under some or all circumstances
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants