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

Build issues with conda-forge package #1199

Closed
ghisvail opened this issue Jun 2, 2021 · 30 comments
Closed

Build issues with conda-forge package #1199

ghisvail opened this issue Jun 2, 2021 · 30 comments
Labels
enhancement Enhancements under development - feel free to join discussion if you'd like to contribute

Comments

@ghisvail
Copy link
Contributor

ghisvail commented Jun 2, 2021

I am in the process of submitting a conda-forge package for ANTs as part of my involvement with the Clinica project.

I got to a point where I have got a good recipe and tried a build by the conda-forge builders. It fails with a bunch of template argument deduction/substitution failed failures, such as:

2021-06-02T13:29:41.4891265Z In file included from $PREFIX/include/ITK-5.2/itkTransform.h:25,
2021-06-02T13:29:41.4891882Z                  from $PREFIX/include/ITK-5.2/itkMatrixOffsetTransformBase.h:24,
2021-06-02T13:29:41.4892524Z                  from $PREFIX/include/ITK-5.2/itkRigid3DTransform.h:22,
2021-06-02T13:29:41.4893149Z                  from $PREFIX/include/ITK-5.2/itkEuler3DTransform.h:22,
2021-06-02T13:29:41.4893776Z                  from $PREFIX/include/ITK-5.2/itkCenteredEuler3DTransform.h:22,
2021-06-02T13:29:41.4894267Z                  from ../ImageRegistration/itkANTSImageTransformation.h:24,
2021-06-02T13:29:41.4894751Z                  from ../ImageRegistration/itkPICSLAdvancedNormalizationToolKit.h:20,
2021-06-02T13:29:41.4895200Z                  from ../Examples/ANTS.cxx:18:
2021-06-02T13:29:41.4896356Z $PREFIX/include/ITK-5.2/itkVariableLengthVector.h:1301:1: note: candidate: 'template<class TExpr1, class TExpr2> typename itk::mpl::EnableIf<itk::Details::op::CanBeMultiplied<TExpr1, TExpr2>, itk::VariableLengthVectorExpression<TExpr1, TExpr2, itk::Details::op::Mult> >::Type itk::operator*(const TExpr1&, const TExpr2&)'
2021-06-02T13:29:41.4897302Z  1301 | operator*(TExpr1 const & lhs, TExpr2 const & rhs)
2021-06-02T13:29:41.4897656Z       | ^~~~~~~~
2021-06-02T13:29:41.4898281Z $PREFIX/include/ITK-5.2/itkVariableLengthVector.h:1301:1: note:   template argument deduction/substitution failed:
2021-06-02T13:29:41.4898799Z In file included from ../Utilities/ReadWriteData.h:10,
2021-06-02T13:29:41.4899236Z                  from ../ImageRegistration/ANTS_affine_registration2.h:32,
2021-06-02T13:29:41.4899698Z                  from ../ImageRegistration/itkANTSImageTransformation.cxx:22,
2021-06-02T13:29:41.4900223Z                  from ../ImageRegistration/itkANTSImageTransformation.h:190,
2021-06-02T13:29:41.4900701Z                  from ../ImageRegistration/itkPICSLAdvancedNormalizationToolKit.h:20,
2021-06-02T13:29:41.4901147Z                  from ../Examples/ANTS.cxx:18:
2021-06-02T13:29:41.4901802Z $PREFIX/include/ITK-5.2/itkImageFileWriter.h:254:88: note:   candidate expects 2 arguments, 1 provided
2021-06-02T13:29:41.4902391Z   254 |   using ImageType = typename std::remove_const<typename std::remove_reference<decltype(*image)>::type>::type;
2021-06-02T13:29:41.4902972Z       |   

The complete build log is available here.

@cookpa
Copy link
Member

cookpa commented Jun 2, 2021

I'm not familiar with building in conda, but it looks like an ITK version compatibility. Would it be possible to do a Superbuild?

Also, if you can check out source code with git (even over https, no need to log in), it will copy accurate version information into the binaries.

@ghisvail
Copy link
Contributor Author

ghisvail commented Jun 2, 2021

but it looks like an ITK version compatibility.

Perhaps something to do with how ITK 5 is built in the Superbuild vs conda-forge?

Would it be possible to do a Superbuild?

The conda-forge package should reuse existing dependencies available in the archive.

if you can check out source code with git

The conda-forge guidelines favor release tarballs over git checkouts.

Right now, I am using the release tarball from the latest tag (v2.3.5).

@gdevenyi
Copy link
Contributor

gdevenyi commented Jun 2, 2021

You'll need to override the version info during build if you use a tarball:
#1183

@cookpa
Copy link
Member

cookpa commented Jun 3, 2021

The version numbering is troublesome. This is definitely something we need to improve.

Building with system ITK is difficult because we tend to keep up to date with ITK development releases. You can get the version ANTs is currently using here

https://github.com/ANTsX/ANTs/blob/master/SuperBuild/External_ITKv5.cmake

Sometimes you can find a nearby minor release of ITK that works.

@ghisvail
Copy link
Contributor Author

ghisvail commented Jun 3, 2021 via email

@ghisvail
Copy link
Contributor Author

What prevents ANTs for being built with a standard release of ITK v5.2?

@cookpa
Copy link
Member

cookpa commented Jul 1, 2021

As well as the version, there's specific options to the ITK build that ANTs needs.

In the SuperBuild, these are set up here

https://github.com/ANTsX/ANTs/blob/master/SuperBuild/External_ITKv5.cmake

in particular see the options here

https://github.com/ANTsX/ANTs/blob/master/SuperBuild/External_ITKv5.cmake#L114-L138

@ghisvail
Copy link
Contributor Author

I have contacted the ITK conda package maintainers, who helped identify a few issues. One may be that the conda compilers now default to C++17 and ITK and other third-party dependencies are built with this standards version.

Did you test any build of ANTs with C++17 in your CI infrastructure (with ITK built with C++17 too)? The template deduction errors here are in core ITK modules, which suggest a C++ standards mismatch.

@cookpa
Copy link
Member

cookpa commented Jul 15, 2021

Thanks for this information. Our CI is pretty minimal. As far as I know, they just do the SuperBuild with the default options, including -std=c++11.

@cookpa
Copy link
Member

cookpa commented Jul 16, 2021

Possibly relevant PR #1209

@cookpa
Copy link
Member

cookpa commented Aug 6, 2021

Further code updates in #1218

@ghisvail
Copy link
Contributor Author

ghisvail commented Aug 6, 2021

It builds fine now on both osx and linux targets. Do I still need ANTSPATH set if everything is installed under PATH already?

@cookpa
Copy link
Member

cookpa commented Aug 6, 2021

The executables don't need it, but it's needed for many of the scripts.

@ghisvail
Copy link
Contributor Author

ghisvail commented Aug 8, 2021

Ok, the good news are that ANTs now builds fine with C++17 on the conda-forge infrastructure with the conda toolchain.

Last details to be sorted concern the availability of a release tarball which contains the most recent fixes and how to properly pass version information to the build system. Could you please tag a new version of ANTs, and provide instructions as to how to set the appropriate version for the conda-forge executables?

Cheers, and thank you to all of you who helped make this happen 👍

@cookpa
Copy link
Member

cookpa commented Aug 8, 2021

All credit to @hjmjohnson

We're overdue for a new release tag, so I will work on that.

@ghisvail
Copy link
Contributor Author

@cookpa do you plan to make a new release soon then?

@gdevenyi
Copy link
Contributor

I think #1216 should be a blocker for another release

@cookpa
Copy link
Member

cookpa commented Aug 16, 2021

Yes, though I think I can fix that relatively quickly. I'll try to work on it this week.

@cookpa
Copy link
Member

cookpa commented Aug 26, 2021

OK #1216 is done. I think we still need to address the version problem. I'll open a new issue

@oesteban
Copy link

oesteban commented Jul 1, 2022

I was wondering whether this https://anaconda.org/aramislab/ants is related to these efforts. What is the status of this issue?

/cc @arokem, who tipped me about the conda package.

@stnava
Copy link
Member

stnava commented Jul 1, 2022

that's interesting - I never heard of that before.

@ghisvail
Copy link
Contributor Author

ghisvail commented Jul 1, 2022

Yes, we used to maintain our own Conda channel for neuroimaging tools we needed for Clinica. We have contributed official conda-forge recipes for some of them, but the efforts for ANTs died out since the release promised last year has yet to happen.

I think the corresponding PR on conda-forge was also closed automatically due to inactivity.

@cookpa cookpa added the enhancement Enhancements under development - feel free to join discussion if you'd like to contribute label Jul 5, 2022
@cookpa
Copy link
Member

cookpa commented Jul 5, 2022

Does conda need ANTs to build against a release version of ITK? I ask because ITK 5.3.0 will be out soon, so if it makes sense to build a release against that, we could do so.

@ghisvail
Copy link
Contributor Author

ghisvail commented Jul 5, 2022

Does conda need ANTs to build against a release version of ITK? I ask because ITK 5.3.0 will be out soon, so if it makes sense to build a release against that, we could do so.

It needs to build against whatever version of ITK is available in their distribution. Today it is 5.2, one day it will be 5.3.

@cookpa
Copy link
Member

cookpa commented Jul 5, 2022

If you're agreeable @stnava @ntustison I shall bump to 2.4.0, since it's been a while and there are quite a few changes.

@stnava
Copy link
Member

stnava commented Jul 5, 2022

yup

@cookpa
Copy link
Member

cookpa commented Jul 6, 2022

@effigies
Copy link

@ghisvail Curious if you're going to take another stab at packaging on conda-forge?

@ghisvail
Copy link
Contributor Author

@ghisvail Curious if you're going to take another stab at packaging on conda-forge?

I think it my PR to conda-forge was closed recently by the stale bot. I could try submitting it again with the latest ANTs version.

@cookpa
Copy link
Member

cookpa commented Jul 19, 2023

Closing this because conda builds are now up and running for Mac and Linux (still need help to get Windows working #1547)

@cookpa cookpa closed this as completed Jul 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancements under development - feel free to join discussion if you'd like to contribute
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants