Disable autodownload by default

[shrit]

Signed-off-by: Omar Shrit omar@shrit.me

[rcurtin]

I agree with the idea here, but I think we'll need to make a few CI changes. ensmallen will need to be installed; I think you should be able to use apt-get, brew, and nuget in the various .ci/ files. There may be a bit of work to be done on the build nodes on Jenkins, but that should be pretty easy.

[shrit]

I will update the ci to download all necessary dependencies 👍

[rcurtin]

I installed libensmallen-dev and libstb-dev on all the CI build nodes I control, so hopefully the Jenkins jobs should now work correctly too. @zoq would you mind installing those packages on the CI nodes of yours? :)

[zoq]

Some or most of the jobs use https://github.com/mlpack/jenkins-conf/blob/master/Dockerfiles/jenkins-amd64-debian.Dockerfile so we only have to update the file and rebuild the docker. I'll update it in a second and push it.

[zoq]

Docker files updated, and images rebuild.

[shrit]

I found it strange that the CI was not able to locate libensmallen-dev maybe I did not write the package name correctly? it seems to be a part of the Ubuntu repository.
Also, there is a lib libstb-dev available for Ubuntu and Debian. Let me know if it has the same headers or, I will just wget it from mlpack website.

[rcurtin]

It looks like the Azure build workers use xenial, which is pretty old and doesn't have libensmallen-dev available (you can see this in the output, but the build doesn't fail). It seems like the easiest solution would be to just install ensmallen from source too. 👍

[rcurtin]

Just left two comments that should hopefully fix the build. 👍

[shrit]

@rcurtin Good news, Armadillo library has been finally found on windows, let us see if there will be no linking issues, if yes then we can merge this pull request 👍

[shrit]

Here is the last error from this pull request,
@rcurtin, I think this error is coming from the armadillo, any possibilities of an ABI incompatibility?

##[error]mlpack.lib(adaboost_model.obj)(0,0): Error LNK2001: unresolved external symbol "class std::mersenne_twister_engine<unsigned __int64,64,312,156,31,-5403634167711393303,29,6148914691236517205,17,8202884508482404352,37,-2270628950310912,43,6364136223846793005> arma::mt19937_64_instance" (?mt19937_64_instance@arma@@3V?$mersenne_twister_engine@_K$0EA@$0BDI@$0JM@$0BP@$0?EKPNJAKFFGJJOGBH@$0BN@$0FFFFFFFFFFFFFFFF@$0BB@$0HBNGHPPPONKGAAAA@$0CF@$0?IBBCAAAAAAAAA@$0CL@$0FIFBPECNEMJFHPCN@@std@@A)

[rcurtin]

Wait a second... mt19937_64_instance is not a symbol that exists in Armadillo 9.800.6! Is it possible that Armadillo is installed twice on the system? I think this might be the case---is the ensmallen nuget package shipping with a bundled Armadillo? I think this could cause a problem if both ENSMALLEN_INCLUDE_DIR and ARMADILLO_INCLUDE_DIR have armadillo_bits/ subdirectories---the compiler might include the wrong one.

If this is correct (please double-check my reasoning above), then I see two options:

1. Remove the Armadillo headers that shipped with ensmallen, so that we are always using Armadillo 9.800.6 in this build.
2. Use the Armadillo headers that shipped with ensmallen, instead of downloading and compiling Armadillo locally.

I think the first solution might be preferable, since it always allows us to build mlpack against the minimum required version of Armadillo.

[shrit]

@rcurtin I have already done that, I suppose that maybe it is not working? I will try to verify if the directory is still there

[rcurtin]

Ah, sorry, I missed that. Well, maybe it is worth printing everything in the ensmallen directory, just to make sure the headers are not there.

[shrit]

What I find strange is that it has never been able to link with any armadillo version including the one that is in ensmallen package. However, when I corrected the path to add the tree\include, it showed behaviour similar as if it was linking with the package.
I have added a tree to see if armadillo is not deleted from the ensmallen package, let us see.

[shrit]

@rcurtin we are sure now that visual studio is trying to use the local headers in the ensmallen package, which is causing all of these issues.
Have a look here
https://dev.azure.com/mlpack/mlpack/_build/results?buildId=7292&view=logs&j=934eac7e-378e-5e05-ed87-f05e34fa9fdc&t=4ee4870b-c6ca-5a83-cdbe-639dd57a3582&l=9973

[rcurtin]

Take a look at the error:

##[error]C:\hostedtoolcache\windows\ensmallen.2.17.0\installed\x64-linux\include\armadillo(21,10): Error C1083: Cannot open include file: 'armadillo_bits/compiler_check.hpp': No such file or directory

I think that you have removed everything of Armadillo that's distributed with ensmallen... except the armadillo file. So if you remove that too, in addition to the armadillo_bits/ directory, then I think we will be closer to something that works. :)

[shrit]

@rcurtin I think it has passed now, finally, I only got out of heap space error, which is normal from time to time.
I restarted the windows build to see if it may pass this time.

[rcurtin]

Everything looks good to me here. Thank you so much for putting so much effort into this! Before merge, I think we should still make sure that the Windows build succeeds at least once (we might have to restart it a few times).

[zoq]

Might be a good idea to clean the commit history or we just do a squash & merge and clean it there.

[zoq]

I'm not sure I get the reason why we download the ensmallen package from nuget, and delete everything besides the header files. In the linux build we just download the header files and copy everything into the target directory. Is there anything that I'm missing here?

[shrit]

Frankly, I do not remember why we went into NuGet direction, there was a strong reason, but I can not remember why

[shrit]

After all of this suffering, I think we can merge this pull request. Whatever the first reason was, it is better to use NuGet for future versions, we will have to change only the number now for ensmallen

[mlpack-bot]

Second approval provided automatically after 24 hours. 👍

[rcurtin]

Still a memory error; I tried restarting the build again.

[shrit]

@rcurtin was it a linking error?

[rcurtin]

Nope, just 'compiler out of heap space'. I'd just like to make sure we can at least get it to build cleanly once. :)

[shrit]

In this case, the issue will be coming from armadillo 9.8 since it is the first time we try to compile mlpack with it on Windows.
I do not know how we can avoid these, maybe cut the tests examples into several units?

[shrit]

All green 💯 🚀

[rcurtin]

Awesome! Great work @shrit, super cool we were finally able to get this one working right. :)

[shrit]

@rcurtin @jeffin143 thanks, I think we are close to merging #3006, if you have any spare time, do not hesitate in giving hints on the Unicode strange loop behaviors