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

Provide conda-forge package #9

Closed
ghisvail opened this issue May 31, 2021 · 17 comments
Closed

Provide conda-forge package #9

ghisvail opened this issue May 31, 2021 · 17 comments

Comments

@ghisvail
Copy link

What's the statuts of (and work remaining for) porting Convert3D to ITK version 5?

I can see some initial work on the itk5 branch and a pending PR #8.

I'd be happy to help.

@pyushkevich
Copy link
Owner

pyushkevich commented May 31, 2021 via email

@ghisvail
Copy link
Author

ghisvail commented Jun 2, 2021

I am trying to build a conda-forge package for convert3d and only ITK 5 is available hence my request.

So far, I am stuck with the MorphologicalContourInterpolation dependency which is not available right now. It is still a ITK Remote module and they are not built by default on conda-forge.

Have you got any plans to merge it to the main branch at some point and tag a new release?

@pyushkevich
Copy link
Owner

pyushkevich commented Jun 2, 2021 via email

@ghisvail ghisvail changed the title Compatibility with ITK 5 Provide conda-forge package Jun 2, 2021
@ghisvail
Copy link
Author

ghisvail commented Jun 2, 2021

I added a CMake option CONVERT3D_USE_ITK_REMOTE_MODULES

Brilliant. Thank you so much.

Progress on the conda-forge package is here btw:

conda-forge/staged-recipes#15090

@ghisvail
Copy link
Author

ghisvail commented Jun 2, 2021

@pyushkevich fyi, you've got a few deprecated usage of VNL:

2021-06-02T13:50:33.0095954Z ../utilities/AffineTransformTool.cxx: In function 'void quart_print(MatrixType&)':
2021-06-02T13:50:33.0103364Z ../utilities/AffineTransformTool.cxx:224:22: warning: 'vnl_matrix_fixed<T, num_rows, num_cols>::operator const vnl_matrix_ref<T>() const [with T = double; unsigned int num_rows = 3; unsigned int num_cols = 3]' is deprecated: Implicit cast conversion is dangerous.\nUSE: .as_matrix() or .as_ref() member function for clarity. [-Wdeprecated-declarations]
2021-06-02T13:50:33.0109852Z   224 |   vnl_qr<double> qr(A);
2021-06-02T13:50:33.0115455Z       |                      ^
2021-06-02T13:50:33.0122988Z In file included from $PREFIX/include/ITK-5.2/vnl/vnl_matrix_fixed.hxx:9,
2021-06-02T13:50:33.0138077Z                  from $PREFIX/include/ITK-5.2/itkMatrix.h:25,
2021-06-02T13:50:33.0139307Z                  from $PREFIX/include/ITK-5.2/itkImageBase.h:34,
2021-06-02T13:50:33.0140319Z                  from $PREFIX/include/ITK-5.2/itkNeighborhoodAccessorFunctor.h:22,
2021-06-02T13:50:33.0141315Z                  from $PREFIX/include/ITK-5.2/itkImage.h:28,
2021-06-02T13:50:33.0142074Z                  from ../itkextras/itkOrientedRASImage.h:4,
2021-06-02T13:50:33.0142942Z                  from ../utilities/AffineTransformTool.cxx:26:
2021-06-02T13:50:33.0143874Z $PREFIX/include/ITK-5.2/vnl/vnl_matrix_fixed.h:687:3: note: declared here
2021-06-02T13:50:33.0144879Z   687 |   operator const vnl_matrix_ref<T>() const { return  this->as_ref(); }
2021-06-02T13:50:33.0145535Z       |   ^~~~~~~~
2021-06-02T13:50:33.0146426Z ../utilities/AffineTransformTool.cxx: In function 'void irtk_write(MatrixStack&, const char*)':
2021-06-02T13:50:33.0148302Z ../utilities/AffineTransformTool.cxx:353:22: warning: 'vnl_matrix_fixed<T, num_rows, num_cols>::operator const vnl_matrix_ref<T>() const [with T = double; unsigned int num_rows = 3; unsigned int num_cols = 3]' is deprecated: Implicit cast conversion is dangerous.\nUSE: .as_matrix() or .as_ref() member function for clarity. [-Wdeprecated-declarations]
2021-06-02T13:50:33.0149572Z   353 |   vnl_qr<double> qr(A);
2021-06-02T13:50:33.0150115Z       |                      ^
2021-06-02T13:50:33.0150942Z In file included from $PREFIX/include/ITK-5.2/vnl/vnl_matrix_fixed.hxx:9,
2021-06-02T13:50:33.0151939Z                  from $PREFIX/include/ITK-5.2/itkMatrix.h:25,
2021-06-02T13:50:33.0152883Z                  from $PREFIX/include/ITK-5.2/itkImageBase.h:34,
2021-06-02T13:50:33.0154181Z                  from $PREFIX/include/ITK-5.2/itkNeighborhoodAccessorFunctor.h:22,
2021-06-02T13:50:33.0155183Z                  from $PREFIX/include/ITK-5.2/itkImage.h:28,
2021-06-02T13:50:33.0156182Z                  from ../itkextras/itkOrientedRASImage.h:4,
2021-06-02T13:50:33.0156924Z                  from ../utilities/AffineTransformTool.cxx:26:
2021-06-02T13:50:33.0158004Z $PREFIX/include/ITK-5.2/vnl/vnl_matrix_fixed.h:687:3: note: declared here
2021-06-02T13:50:33.0159361Z   687 |   operator const vnl_matrix_ref<T>() const { return  this->as_ref(); }
2021-06-02T13:50:33.0160026Z       |   ^~~~~~~~

@ghisvail
Copy link
Author

ghisvail commented Jun 2, 2021

There is also an occurrence of: FIND_LIBRARY(FFTW_LIB fftw3f) but it looks like it's not used anywhere directly and the conda-forge tooling complains of overlinkage.

Aren't FFT features provided by ITK in your case?

@ghisvail
Copy link
Author

ghisvail commented Jun 2, 2021

Apart from that, the binaries build fine on the conda-forge infrastructure so we are quite close modulo the overlinking issue.

@pyushkevich
Copy link
Owner

pyushkevich commented Jun 2, 2021 via email

@ghisvail
Copy link
Author

ghisvail commented Jun 2, 2021

Actually, you've got and explicit dependency on FFTW here.

@pyushkevich
Copy link
Owner

pyushkevich commented Jun 3, 2021 via email

@ghisvail
Copy link
Author

ghisvail commented Jun 3, 2021

Thanks, I am rebasing the package onto your latest commit.

Do you want to be added as co-maintainer of the conda-forge repo?

@pyushkevich
Copy link
Owner

pyushkevich commented Jun 3, 2021 via email

@ghisvail
Copy link
Author

ghisvail commented Jun 3, 2021 via email

@ghisvail
Copy link
Author

ghisvail commented Jun 7, 2021

The conda-forge package is ready for review. @pyushkevich would you mind tagging the current version of the code with a new one (v1.3.0?) so I can reference a proper release tarball in the conda-forge recipe?

Cheers.

@pyushkevich
Copy link
Owner

pyushkevich commented Jun 7, 2021 via email

@hubutui
Copy link

hubutui commented Mar 7, 2022

I check the conda-forge recipe, and found:

export CFLAGS="${CFLAGS} -I ${PREFIX}/include/eigen3"
export CXXFLAGS="${CXXFLAGS} -I ${PREFIX}/include/eigen3"

so, why not add -I ${PREFIX}/include/eigen3 to https://github.com/pyushkevich/c3d/blob/master/CMakeLists.txt?
add this line:

set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -I/usr/include/eigen3")

to https://github.com/pyushkevich/c3d/blob/master/CMakeLists.txt line 18.

@ghisvail
Copy link
Author

Forgot to close this issue. A conda package has been available for a while now.

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

No branches or pull requests

3 participants