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

Use LBFGS++ with Spectra #13

Closed
drelatgithub opened this issue Aug 4, 2020 · 2 comments
Closed

Use LBFGS++ with Spectra #13

drelatgithub opened this issue Aug 4, 2020 · 2 comments

Comments

@drelatgithub
Copy link

drelatgithub commented Aug 4, 2020

Thank you for making these two amazing libraries! Currently, I would like to the two libraries in my project. However, if I use both LBFGS++ and Spectra in a single translation unit, I would encounter compiling errors saying that symbol BKLDLT is undefined. A minimum code example to reproduce the problem is

#include "Spectra/MatOp/DenseSymShiftSolve.h"
#include "LBFGSpp/LBFGS.h"

I guess that the problem I encountered is due to the same include guard name BK_LDLT_H in the files named BKLDLT.h presented in both libraries as header dependencies. The library included later would see its BKLDLT.h as an almost empty file because the library included earlier already defines BK_LDLT_H. Normally that would be okay, because the contents in one file are simply reused for both libraries, but in this case the 2 BKLDLT.h files are almost identical except that they define class template BKLDLT in different namespaces, which makes the file in one library unusable for the other.

The problem goes away if I rename the include guard macros to make them different (such as LBFGSPP_BK_LDLT_H and SPECTRA_BK_LDLT_H). IMO this makes the header only libraries more separated and makes the include guard collisions highly unlikely. I'm curious to know what you think!

@yixuan
Copy link
Owner

yixuan commented Aug 4, 2020

Hi @drelatgithub, thanks for pointing this out. You are right, and that's why I have added extra prefix in this commit: yixuan/spectra@ce79ea5.

This commit belongs to a development branch (https://github.com/yixuan/spectra/tree/1.y.z), so it is not in the master branch yet.

@drelatgithub
Copy link
Author

Thanks for the reply! I didn't know that the changes were already there! Guess I'll sit back and look forward to the new release!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants