Backend build fails in master

[theq629]

$ cd domains/wikipediahistory
$ make
SRC=../../backend CONFIG=. OUT=backend make -f ../../backend/Makefile build
make[1]: Entering directory '/home/mwhitney/projects/lensingwikipedia/repo/domains/wikipediahistory'
find ../../backend -maxdepth 1 \( -name '*.py' -o -executable \) \( -not -type d \) -exec cp {} backend \;
find . -maxdepth 1 -name '*.py' -exec cp {} backend \;
cp -r ../../backend/bhtsne backend/
if grep --quiet "Ubuntu" /etc/lsb-release; then \
        ln -s /usr/lib/libcblas.so backend/bhtsne/libcblas.so; \
else \
        ln -s /usr/lib64/atlas/libcblas.so backend/bhtsne/libcblas.so; \
fi
grep: /etc/lsb-release: No such file or directory
ln: failed to create symbolic link ‘backend/bhtsne/libcblas.so’: File exists
../../backend/Makefile:10: recipe for target 'build' failed
make[1]: *** [build] Error 1
make[1]: Leaving directory '/home/mwhitney/projects/lensingwikipedia/repo/domains/wikipediahistory'
Makefile:12: recipe for target 'backend' failed
make: *** [backend] Error 2

[theq629]

This doesn't actually stop the build finishing, so maybe it doesn't really fail. Why do we need to link the library locally, though?

[avacariu]

I'm not sure, that's how @KonceptGeek wrote it. The grep part is my mistake, though. I didn't realize it'd fail when there's no file.

I'll fix it in a second.

[avacariu]

It looks like it's actually fine. Grep saying No such file or directory is only an annoyance but the if statement still works as before. It's just that grep ignores the --quiet flag in that situation.

I'll still do a PR to check if the file exists before grepping it, though, just to get rid of that message.

The actual failure seems to be with ln. Maybe try to do a make clean first?

[theq629]

It's still not going to get the library on any system (including mine and any without lib64) that doesn't have /usr/lib64/atlas/libcblas.so, though. I think on most GNU systems it would be possible to get the right path by doing ldconfig -p and then grepping for the library, but I don't know how portable that is either.

[theq629]

I'm still wondering if we need to have a local link to the library at all, though. It seems to build fine if I take that part out, and then there aren't any system-specific assumptions.

[avacariu]

RE: ldconfig If it'll work fine on CentOS and Ubuntu, then it's probably portable enough for what we need.

RE: removing symlink If it builds and executes properly (and ld finds the library properly) on both CentOS and Ubuntu without the symlink, then it's probably best we remove it completely. I'll play around with it this evening (London time) and see what I can do (unless you're going to take a look at it first). I remember having some issues with the symlink in the past (which is why I added the Ubuntu-specific symlink), but I don't quite remember what the error messages were.

[theq629]

I just tried on a lab machine without any special configuration, and it found the library but not header files -- but I assume I'd need to load a module for cblas to make it work.

Anyway, I feel like hardcoding library file paths is generally a bad idea, and risks breaking the build on properly configured systems. It will also cause problems with modules or anywhere the system might intend to use a library in a non-default location.

So if we do need to handle special cases that need the link, what about checking for the library first and then fall back to special cases if that fails, something like this:

if ! ld -l blas; then \
        if grep --quiet "Ubuntu" /etc/lsb-release; then \
                ln -s /usr/lib/libcblas.so backend/bhtsne/libcblas.so; \
        elif some check for another system \
                ln -s /usr/lib64/atlas/libcblas.so backend/bhtsne/libcblas.so; \
        else \
                echo "can't find blas library" 1>&2 \
        fi \
fi

Alternatively, we could check for an environment variable and then provide instructions on how to set it for the systems that need it:

if -n "$(BLASLIBPATH)"; then \
        ln -s $(BLASLIBPATH)" backend/bhtsne/libcblas.so \
fi

[theq629]

Alternatively, if we do move the domains/ stuff to backend/ then we could have an optional Makefile.config that is sourced from the main makefile, and then provide eg Makefile.ubuntu to copy to Makefile.config on systems that need it.

[anoopsarkar]

There is an even bigger problem: sometimes tsne crashes while adding information to the whoosh index. Perhaps we can configure a stable and unstable version of the backend? Linking to C++ libraries and using C++ code while updating the backend index makes tsne inherently unstable and we could use it for some demos, but not deploy to a live site. What do all of you think?

[theq629]

Probably we should have a separate issue for the tsne crash, but we could definitely have a unstable background. But in that case should we just have complete separate stable and unstable branches?

[theq629]

Another build error that doesn't actually stop the build, this one on rebuilding:

mkdir backend/Temp
mkdir: cannot create directory ‘backend/Temp’: File exists
../../backend/Makefile:10: recipe for target 'build' failed
make[1]: *** [build] Error 1
make[1]: Leaving directory '/home/mwhitney/projects/lensingwikipedia/repo/domains/wikipediahistory'
Makefile:12: recipe for target 'backend' failed
make: *** [backend] Error 2

[avacariu]

Hmm, that mkdir on line 21 should really be a mkdir -p.

[avacariu]

I'm not sure I understand the stable and unstable backend issue. What's making it unstable? (I haven't really looked at how the tsne code works)

[avacariu]

I created a PR to fix that, but should we even be creating that directory in the Makefile or should the tsne code be creating its temp directories itself (maybe under /tmp)?

[theq629]

It seems like maybe tsne should have its own makefile that gets called recursively. And probably C++ code and anything in a temp directory shouldn't go in $(OUT).

[theq629]

My proposal for the backend makefile changes is in mwhitneybackendmakefileproposal. It has:

• local makefile settings
• bhtsne has its own makefile
• bhtsne source files aren't copied to output

For the first part, the backend makefile sources Makefile.local (if it exists) from the directory make is run from. This is the same way the frontend makefiles used to work. So if someone needs to set the cblas path, they set LD_LIBRARY_PATH in Makefile.local. If common systems need that set, then we supply eg Makefile.ubuntu which can be copied to Makefile.local.

[anoopsarkar]

@KonceptGeek can you test this out? if it works for you then we can merge a PR to master.

[theq629]

I updated my proposal branch to work with the current master. #181 already added Makefile.local support.

I'm fairly sure that @KonceptGeek will just need to set LD_LIBRARY_PATH in Makefile.local as follows:

LD_LIBRARY_PATH:=/usr/lib64/atlas/libcblas.so:$(LD_LIBRARY_PATH)

If for some reason LD_LIBRARY_PATH doesn't work, we can add a specific environment variable for the path to libcblas.

[anoopsarkar]

I found why tsne was crashing. Any set of examples greater than 80,000 does not work (without PCA or with PCA, it doesn't matter). I will push a fix to master that ignores all events after the first 80K events. Not ideal, but will ensure index building will not crash.

[theq629]

The original issue has been addressed by my changes getting merged through the docker merge. @anoopsarkar, if the too many points for tsne issue is still a problem then let's make a new issue for it.