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: Add itkAnalyticSignalImageFilter and itkRegionFromReferenceImageFilter wrappings #139

Merged
merged 1 commit into from May 22, 2021

Conversation

tbirdso
Copy link
Contributor

@tbirdso tbirdso commented Apr 28, 2021

Exposes itkAnalyticSignalImageFilter and itkRegionFromReferenceImageFilter for illustration in example notebook.

@tbirdso tbirdso requested review from thewtex and dzenanz April 28, 2021 20:39
@tbirdso
Copy link
Contributor Author

tbirdso commented Apr 29, 2021

Seeing a wrapping issue dealing with the std::complex type:

D:\a\im\include\itkAnalyticSignalImageFilter.hxx(219): error C2678: binary '*': no operator found which takes a left-hand operand of type 'std::complex' (or there is no acceptable conversion)
D:\a\im\include\itkAnalyticSignalImageFilter.hxx(219): note: while trying to match the argument list '(std::complex, double)'

Line 219 is trying to multiple a complex and a scalar, which is defined under <complex>:

outputIt.Set(inputIt.Get() * static_cast(2));

This issue does not cause the cxx build to fail, only the Python wrapping generation for AnalyticSignalImageFilter. I'll also note that this doesn't seem to fail when building with MSBuild, only with GNinja.

@dzenanz @thewtex Are there any additional steps we need to take for wrapping classes dealing with complex numbers? Headers to include?

@dzenanz
Copy link
Member

dzenanz commented Apr 29, 2021

itkBModeImageFilter: warning(4): ITK type not wrapped, or currently not known: itk::FrequencyDomain1DImageFilter< itk::CurvilinearArraySpecialCoordinatesImage< std::complex< double >, 2 >, itk::CurvilinearArraySpecialCoordinatesImage< std::complex< double >, 2 > >

We should use newer packages for this.

../../../include/itkVnlForward1DFFTImageFilter.hxx: In instantiation of ‘void itk::VnlForward1DFFTImageFilter<TInputImage, TOutputImage>::GenerateData() [with TInputImage = itk::Image<double, 4>; TOutputImage = itk::Image<std::complex<float>, 4>]’:
../../../include/itkVnlForward1DFFTImageFilter.hxx:38:1:   required from here
../../../include/itkVnlForward1DFFTImageFilter.hxx:100:11: error: cannot convert ‘std::complex<double>’ to ‘const PixelType&’ {aka ‘const std::complex<float>&’}

To reproduce the error in C++, instantiate itk::VnlForward1DFFTImageFilter<itk::Image<double, 4>, itk::Image<std::complex<float>, 4>>. Python wrapping instantiates this, which causes problems.

@dzenanz
Copy link
Member

dzenanz commented Apr 29, 2021

Python packages have additional wrappings enabled, compared to defaults. The most recent commit which has made it into 5.2.0.post1:
InsightSoftwareConsortium/ITKPythonPackage@8c0dd73
Having these enabled locally should allow you to reproduce the errors.

@tbirdso
Copy link
Contributor Author

tbirdso commented Apr 29, 2021

Thanks @dzenanz . This will require rebuilding ITK locally with the correct wrappings, right?

@dzenanz
Copy link
Member

dzenanz commented Apr 29, 2021

This will require rebuilding ITK locally with the correct wrappings

Correct.

@tbirdso
Copy link
Contributor Author

tbirdso commented May 3, 2021

After successfully rebuilding ITK I'm seeing new errors in itkResampleImageFilter.hxx when building the ITKUltrasound master branch:

C:\repos\ITK\Modules\Filtering\ImageGrid\include\itkResampleImageFilter.hxx(474,20): error C2672: 'itk::CurvilinearArraySpecialCoordinatesImage<PixelType,3>::TransformPhysicalPointToContinuousIndex': no matching overloaded function found
C:\repos\ITK\Modules\Filtering\ImageGrid\include\itkResampleImageFilter.hxx(475,1): error C2780: 'bool itk::CurvilinearArraySpecialCoordinatesImage<PixelType,3>::TransformPhysicalPointToContinuousIndex(const itk::Point<TCoordRep,3> &,itk::ContinuousIndex<TIndexRep,3> &) const': expects 2 arguments - 1 provided
12>

The line in question in itkResampleImageFilter.hxx is here:

const auto transformIndex = [outputPtr, transformPtr, inputPtr](const IndexType & index) {
return inputPtr->template TransformPhysicalPointToContinuousIndex(
transformPtr->TransformPoint(outputPtr->template TransformIndexToPhysicalPoint(index)));
};

Given that ITKUltrasound CI is green and ITK built successfully I'm inclined to believe this is a configuration issue on my side. I have ITK_WRAP_double and ITK_WRAP_complex_double enabled. Are there other configurations that could cause this issue where the base function overload is not found?

@tbirdso
Copy link
Contributor Author

tbirdso commented May 12, 2021

Rebased on master. Build+wrap is successful on Linux, still unclear on why my local Windows build is failing but as CI for master is passing I don't believe we should consider it a blocking issue.

@tbirdso
Copy link
Contributor Author

tbirdso commented May 12, 2021

build-linux-python-packages failure similar to HASI Issue 56.

+ rm dist/itk_bsplinegradient-0.2.4-cp37-cp37m-linux_x86_64.whl
+ compgen -G 'dist/itk*-linux*.whl'
~/work/ITKUltrasound/ITKUltrasound
rm: cannot remove './ITKBSplineGradient/doxygen-1.8.11.linux.bin.tar.gz': No such file or directory
Error: Process completed with exit code 1.

Are we calling rm in the right directory?

@tbirdso
Copy link
Contributor Author

tbirdso commented May 18, 2021

Rebased on #144.

@dzenanz
Copy link
Member

dzenanz commented May 19, 2021

It looks like another conflict-resolving rebase is needed.

Copy link
Member

@thewtex thewtex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📗

@thewtex thewtex merged commit 60dc2ed into KitwareMedical:master May 22, 2021
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.

None yet

3 participants