Closed grlee77 closed 8 years ago
Hi! This is the friendly automated conda-forge-linting service.
I just wanted to let you know that I linted all conda-recipes in your PR (recipe
) and found it was in an excellent condition.
Is there some reason not to install both static and dynamic libraries?
I am now using SuiteSparse_config.mk as in master, but it no longer seems necessary to have separate files for linux and OS X.
Yeah, I started to make them pretty similar at the end. For the most part, started using a bunch of ?=
instead of =
allowed most of the configuration to happen in build.sh
, which seemed preferable anyways. I emailed Tim Davis making this exact suggestion though I don't know if he will go for it.
I changed the license info in meta.yaml slightly. I moved LGPL v2 first as that is what the majority of the code seems to be licensed under (I only found one library with a BSD 3-clause license). In the v4.5.3 source the licenses are more clearly stated in the separate /Doc/License.txt within each library's subfolder.
Yeah, that seems reasonable to me. Licensing here is a bit of a pain unfortunately. Appreciate that you added Apache 2.0 for METIS.
Adding cmake to the build dependencies was needed for building the included Metis v5.1.0.
👍
As far as the static libraries, they do also get built, but I didn't see a way to get make install
to install them.
After the make completes, I can see static libraries in various subfolders, e.g. for CHOLMOD the static lib is at ${SRC_DIR}/CHOLMOD/Lib/libcholmod.a
. Obviously we could manually cp them, but maybe there is a way to do it from the Makefile that I overlooked?
CI is now passing. will try removing the patch and adding install for the static libs
Seems there is an issue copying the static libs over. Here is part of the CircleCI log.
cp: cannot stat `/feedstock_root/build_artefacts/work/SuiteSparse/CHOLAMD/Lib/libcholamd.a': No such file or directory
Command failed: /bin/bash -x -e /recipe_root/build.sh
I see there is a PR at staged-recipes for a METIS package as well. How do we make sure the METIS compiled here doesn't end up conflicting with that one?
Apparently the SPQR_TBB.patch wasn't needed (not sure why I needed it to build successfully on my local linux system).
I see there is a PR at staged-recipes for a METIS package as well. How do we make sure the METIS compiled here doesn't end up conflicting with that one?
I'm not sure how much we should push that one. My concern is the licensing with it is totally opaque. Also, we will want to move to SuiteSpare 4.5.x eventually, which includes a copy/version of METIS that has clear licensing. In any event, let's cross that bridge when we get there.
Apparently the SPQR_TBB.patch wasn't needed (not sure why I needed it to build successfully on my local linux system).
Interesting. It does seem to be working. What version of Linux were you using?
Basically, this is looking really good. Thanks for doing this @grlee77. I'll try to find some time tomorrow or Friday to give it a closer look.
I just noticed that the linux case is still passing -fopenmp
. I can fix that.
Interesting. It does seem to be working. What version of Linux were you using?
Ubuntu 14.04. I didn't look into it much, but the errors reported were:
spqr_parallel.o: In function `spqr_zippy<std::complex<double> >::~spqr_zippy()':
spqr_parallel.cpp:(.text._ZN10spqr_zippyISt7complexIdEED2Ev[_ZN10spqr_zippyISt7complexIdEED5Ev]+0x3): undefined reference to `vtable for tbb::task'
spqr_parallel.o: In function `spqr_zippy<double>::~spqr_zippy()':
spqr_parallel.cpp:(.text._ZN10spqr_zippyIdED2Ev[_ZN10spqr_zippyIdED5Ev]+0x3): undefined reference to `vtable for tbb::task'
spqr_parallel.o: In function `spqr_zippy<std::complex<double> >::~spqr_zippy()':
spqr_parallel.cpp:(.text._ZN10spqr_zippyISt7complexIdEED0Ev[_ZN10spqr_zippyISt7complexIdEED0Ev]+0x3): undefined reference to `vtable for tbb::task'
spqr_parallel.o: In function `spqr_zippy<double>::~spqr_zippy()':
spqr_parallel.cpp:(.text._ZN10spqr_zippyIdED0Ev[_ZN10spqr_zippyIdED0Ev]+0x3): undefined reference to `vtable for tbb::task'
spqr_parallel.o: In function `spqr_zippy<std::complex<double> >::execute()':
spqr_parallel.cpp:(.text._ZN10spqr_zippyISt7complexIdEE7executeEv[_ZN10spqr_zippyISt7complexIdEE7executeEv]+0x7e): undefined reference to `tbb::internal::allocate_child_proxy::allocate(unsigned long) const'
spqr_parallel.cpp:(.text._ZN10spqr_zippyISt7complexIdEE7executeEv[_ZN10spqr_zippyISt7complexIdEE7executeEv]+0xca): undefined reference to `tbb::task::spawn_and_wait_for_all(tbb::task_list&)'
spqr_parallel.o: In function `spqr_zippy<double>::execute()':
spqr_parallel.cpp:(.text._ZN10spqr_zippyIdE7executeEv[_ZN10spqr_zippyIdE7executeEv]+0x7e): undefined reference to `tbb::internal::allocate_child_proxy::allocate(unsigned long) const'
spqr_parallel.cpp:(.text._ZN10spqr_zippyIdE7executeEv[_ZN10spqr_zippyIdE7executeEv]+0xca): undefined reference to `tbb::task::spawn_and_wait_for_all(tbb::task_list&)'
spqr_parallel.o: In function `void spqr_parallel<double>(long, int, spqr_blob<double>*)':
spqr_parallel.cpp:(.text._Z13spqr_parallelIdEvliP9spqr_blobIT_E[_Z13spqr_parallelIdEvliP9spqr_blobIT_E]+0x2e): undefined reference to `tbb::task_scheduler_init::initialize(int, unsigned long)'
spqr_parallel.cpp:(.text._Z13spqr_parallelIdEvliP9spqr_blobIT_E[_Z13spqr_parallelIdEvliP9spqr_blobIT_E]+0x38): undefined reference to `tbb::internal::allocate_root_proxy::allocate(unsigned long)'
spqr_parallel.cpp:(.text._Z13spqr_parallelIdEvliP9spqr_blobIT_E[_Z13spqr_parallelIdEvliP9spqr_blobIT_E]+0x77): undefined reference to `tbb::task_scheduler_init::terminate()'
spqr_parallel.cpp:(.text._Z13spqr_parallelIdEvliP9spqr_blobIT_E[_Z13spqr_parallelIdEvliP9spqr_blobIT_E]+0x92): undefined reference to `tbb::task_scheduler_init::terminate()'
spqr_parallel.o: In function `void spqr_parallel<std::complex<double> >(long, int, spqr_blob<std::complex<double> >*)':
spqr_parallel.cpp:(.text._Z13spqr_parallelISt7complexIdEEvliP9spqr_blobIT_E[_Z13spqr_parallelISt7complexIdEEvliP9spqr_blobIT_E]+0x2e): undefined reference to `tbb::task_scheduler_init::initialize(int, unsigned long)'
spqr_parallel.cpp:(.text._Z13spqr_parallelISt7complexIdEEvliP9spqr_blobIT_E[_Z13spqr_parallelISt7complexIdEEvliP9spqr_blobIT_E]+0x38): undefined reference to `tbb::internal::allocate_root_proxy::allocate(unsigned long)'
spqr_parallel.cpp:(.text._Z13spqr_parallelISt7complexIdEEvliP9spqr_blobIT_E[_Z13spqr_parallelISt7complexIdEEvliP9spqr_blobIT_E]+0x77): undefined reference to `tbb::task_scheduler_init::terminate()'
spqr_parallel.cpp:(.text._Z13spqr_parallelISt7complexIdEEvliP9spqr_blobIT_E[_Z13spqr_parallelISt7complexIdEEvliP9spqr_blobIT_E]+0x92): undefined reference to `tbb::task_scheduler_init::terminate()'
spqr_parallel.o:(.data.rel.ro._ZTI10spqr_zippyIdE[_ZTI10spqr_zippyIdE]+0x10): undefined reference to `typeinfo for tbb::task'
spqr_parallel.o:(.data.rel.ro._ZTI10spqr_zippyISt7complexIdEE[_ZTI10spqr_zippyISt7complexIdEE]+0x10): undefined reference to `typeinfo for tbb::task'
spqr_parallel.o:(.data.rel.ro._ZTV10spqr_zippyIdE[_ZTV10spqr_zippyIdE]+0x28): undefined reference to `tbb::task::note_affinity(unsigned short)'
spqr_parallel.o:(.data.rel.ro._ZTV10spqr_zippyISt7complexIdEE[_ZTV10spqr_zippyISt7complexIdEE]+0x28): undefined reference to `tbb::task::note_affinity(unsigned short)'
collect2: error: ld returned 1 exit status
Looks like the OpenMP fix is working.
Is it possible the wrong compiler was getting picked up on Ubuntu 14.04? There are complaints like undefined reference to vtable
, which look a bit strange.
I think the last two commits have addressed the remaining comments.
I can squash a bunch of the commits if you want a cleaner history here.
I see that you already pinned cvxopt
to use v4.4.* so merging this shouldn't break anything. Then we can test the scikit-umfpack
PR over at staged-recipes.
LGTM. Thanks @grlee77. Let's put it through its paces. 🏇
I cleaned up conda-forge/staged-recipes#682 which builds shared SuiteSparse libraries compatible with the open scikit-umfpack PR. Sstatic libraries actually get built as well, but
make install
doesn't install them.I am now using SuiteSparse_config.mk as in master, but it no longer seems necessary to have separate files for linux and OS X.
I changed the license info in meta.yaml slightly. I moved LGPL v2 first as that is what the majority of the code seems to be licensed under (I only found one library with a BSD 3-clause license). In the v4.5.3 source the licenses are more clearly stated in the separate /Doc/License.txt within each library's subfolder.
Adding cmake to the build dependencies was needed for building the included Metis v5.1.0.