superlu
superlu copied to clipboard
These are some improvements to SuperLU to try to make it more cross p…
…latform and facilitate packaging.
These improvements are:
- Force the built in blas library to be compiled as static instead of shared. This is so it can linked into the shared library in Windows and probably other Operating Systems. The internal cblas library can NOT be built as a a shared library by itself.
- Remove the assumption that the SuperLU library shared run-time library will be prefixed with lib and have a “.so” or “.a” file extension. In Windows, the shared run-time library has a .dll extension. In Windows Visual C, a prefix such as “lib” is not used and the extension is “.lib”, not “.a” In Cygwin and MSYS2, the run-time .dll prefix is not “lib” at all. It is “cyg” and “msys-“ respectively. Making this more complicated is that in Windows, you do not link directly with the shared-library run-time file but rather, an import library. In MINGW-W64, CygWin, and MSYS, that has the extension, “.dll.a”.
- Another issue is that in Windows 64, unlike many Unix-like Operating Systems, the int, uint, and long do not expand to 8 bits in the 64-bit version of Windows, Those types are still 4 bytes wide which is not wide enough for an 8-byte sized pointer. To fix this, I had to change int to ptrdiff_t in ExpHeader and LU_stack_t in slu_util.h. For sizes, the size_t really should be used and for pointers, a pointer should be used. GCC will rightfully warn you about this issue.
- I implemented building doxygen documentation into the cmake build process and Cmake can also install the documentation. You now have the option of building the SuperLU documentation as .HTML or the traditional man pages as well as both depending upon packaging needs.
- I tried fixing some doxygen warnings. I use Doxygen 1.8.14 along with Grapviz to generate diagrams. I made a Doxyfile.in that generates a Doxyfile in the build directory if enable_docs is on. I also use Doxygen to create to .HTML and Unix man documentation.
- The make.inc file generated by cmake is copied to the build directories. The thing is that sometimes, you might be building several different sets of binaries for i686 (32-bit) and (x86_64) 64-bit as well as a static and shared version for inclusion in packages. IOW, I am generating 4 sets of SuperLU binary files. Actually, I think the Makefile build system is not adequate for building stuff on Windows using GCC because you use a command such as “g++ -shared -o example_dll.dll example_dll.o -Wl,--out-implib,libexample_dll.a”
- In Windows, it’s usually a good idea to embed version information right into the .dll run-time so that if you right click it, you can get information such as the copyright and .DLL version in Windows explorer.
- I had to fix the library install so that the .dll file is always installed in the bin directory instead of the lib directory. Windows will search first for .dll’s in the same directory as the executable. MINGW, Cygwin, and MSYS packages also place the .DLL’s in the appropriate directory to take advantage of this.
- When building the interal CBLAS library, Cmake will check for the f2c package and fail if that is not located.
- I like your changes and I would like to see them accepted and integrated.
- You are addressing a couple of important, but different things. Every bullet point above should be a merge request on its own. That make the author of SuperLU easier to review, test, and integrate. And it also make easier to reject an idea from you. Take for example b466c2276d3f51: Not everybody cares about line endings. I would accept your commit, but it changes a lot of files and line and makes using git blame harder. So I would understand if it is not accepted.
Some comments regarding your individual commits:
- 3f2dbef85: You have nine different point within your commit message. That is a clear sign you should split this commit into at least nine commits, probably even more. And make nine merge request out of them.
- b466c227: Spelling mistakes in the first line of commit message. Again, MR of its own.
- 0cb29b9: Including untested code isn't a good idea. Maybe drop it altogether.
- 47595: Again, you are mixing a couple of different things. Split them into separate commits and merge requests. Having doxygen warnings fix, will be accepted in no-time!
Disclaimer: I am just a random guy in the internet. My opinion might differ from the author of SuperLU. But I like the changes and want to give my advice to get them included. If you feel unsure, please ask for more details. I am willing to help you getting things included and splitting your commits.