c3d
c3d copied to clipboard
Provide conda-forge package
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.
Branch itk5 is fully functional. It works with 5.1.2. Please let me know if you encounter any issues.
On Mon, May 31, 2021 at 5:16 AM Ghislain Antony Vaillant < @.***> wrote:
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 https://github.com/pyushkevich/c3d/pull/8.
I'd be happy to help.
— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/pyushkevich/c3d/issues/9, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJPEW2RISFC6XLUIRWESYLTQNHXRANCNFSM452QFQRQ .
-- Paul A. Yushkevich, Ph.D. Professor of Radiology Penn Image Computing and Science Laboratory University of Pennsylvania Perelman School of Medicine
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?
Hi Ghislain
Having c3d in conda would be great, and if you could also do the same four our greedy registration tool (https://github.com/pyushkevich/greedy), that would be great too.
I added a CMake option CONVERT3D_USE_ITK_REMOTE_MODULES (which is ON by default). If you set it to OFF, it will no longer look for ITK remote modules. I also merged itk5 branch into the main one.
Thanks, Paul
On Wed, Jun 2, 2021 at 5:32 AM Ghislain Antony Vaillant < @.***> wrote:
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?
— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/pyushkevich/c3d/issues/9#issuecomment-852869811, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJPEWZPCOKJYSZD7CK5XATTQX3ERANCNFSM452QFQRQ .
-- Paul A. Yushkevich, Ph.D. Professor of Radiology Penn Image Computing and Science Laboratory University of Pennsylvania Perelman School of Medicine
I added a CMake option CONVERT3D_USE_ITK_REMOTE_MODULES
Brilliant. Thank you so much.
Progress on the conda-forge package is here btw:
https://github.com/conda-forge/staged-recipes/pull/15090
@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 | ^~~~~~~~
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?
Apart from that, the binaries build fine on the conda-forge infrastructure so we are quite close modulo the overlinking issue.
Hi, just pushed a commit removing the FFTW line from CMakeLists
Thanks!
On Wed, Jun 2, 2021 at 10:06 AM Ghislain Antony Vaillant < @.***> wrote:
Apart from that, the binaries build fine on the conda-forge infrastructure so we are quite close modulo the overlinking issue.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/pyushkevich/c3d/issues/9#issuecomment-853057948, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJPEW2Y72QHJ3TEKRFTA7TTQY3GLANCNFSM452QFQRQ .
-- Paul A. Yushkevich, Ph.D. Professor of Radiology Penn Image Computing and Science Laboratory University of Pennsylvania Perelman School of Medicine
Actually, you've got and explicit dependency on FFTW here.
Thanks, I missed that. The file is not used, and I just pushed a commit deleting it.
On Wed, Jun 2, 2021 at 5:53 PM Ghislain Antony Vaillant < @.***> wrote:
Actually, you've got and explicit dependency on FFTW here https://github.com/pyushkevich/c3d/blob/master/adapters/SimpleElasticRegistration.h .
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/pyushkevich/c3d/issues/9#issuecomment-853408390, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJPEW2ZHEDWJ2YKL633WCDTQ2R5ZANCNFSM452QFQRQ .
-- Paul A. Yushkevich, Ph.D. Professor of Radiology Penn Image Computing and Science Laboratory University of Pennsylvania Perelman School of Medicine
Thanks, I am rebasing the package onto your latest commit.
Do you want to be added as co-maintainer of the conda-forge repo?
Thanks, sure, I don't really know how to package stuff for conda though.
On Thu, Jun 3, 2021 at 10:54 AM Ghislain Antony Vaillant < @.***> wrote:
Thanks, I am rebasing the package onto your latest commit.
Do you want to be added as co-maintainer of the conda-forge repo?
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/pyushkevich/c3d/issues/9#issuecomment-853932247, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJPEW3PDNYIRL5FQWM3OETTQ6JQVANCNFSM452QFQRQ .
-- Paul A. Yushkevich, Ph.D. Professor of Radiology Penn Image Computing and Science Laboratory University of Pennsylvania Perelman School of Medicine
That's the nice thing, once it's done the only maintenance effort is just merging software upgrade PRs proposed by the conda-forge CI.
Le jeu. 3 juin 2021 à 19:21, Paul Yushkevich @.***> a écrit :
Thanks, sure, I don't really know how to package stuff for conda though.
On Thu, Jun 3, 2021 at 10:54 AM Ghislain Antony Vaillant < @.***> wrote:
Thanks, I am rebasing the package onto your latest commit.
Do you want to be added as co-maintainer of the conda-forge repo?
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/pyushkevich/c3d/issues/9#issuecomment-853932247, or unsubscribe < https://github.com/notifications/unsubscribe-auth/AAJPEW3PDNYIRL5FQWM3OETTQ6JQVANCNFSM452QFQRQ
.
-- Paul A. Yushkevich, Ph.D. Professor of Radiology Penn Image Computing and Science Laboratory University of Pennsylvania Perelman School of Medicine
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/pyushkevich/c3d/issues/9#issuecomment-854044491, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAO7U33I4GPT4DW2EZY5JP3TQ62ZXANCNFSM452QFQRQ .
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.
Thanks, Ghislain! I just tagged v1.3.0 and pushed the tag.
On Mon, Jun 7, 2021 at 12:25 PM Ghislain Antony Vaillant < @.***> wrote:
The conda-forge package is ready for review. @pyushkevich https://github.com/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.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/pyushkevich/c3d/issues/9#issuecomment-856081661, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJPEW2MR7XAXXTX4WFAOJTTRTXHZANCNFSM452QFQRQ .
-- Paul A. Yushkevich, Ph.D. Professor of Radiology Penn Image Computing and Science Laboratory University of Pennsylvania Perelman School of Medicine
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.
Forgot to close this issue. A conda package has been available for a while now.