Add ABI version handling in C code and Python code #323
Conversation
pygpu.gpuarray.abi_version() method. A new header file (abi_version.h) auto-generated by CMake has been added.
src/gpuarray/abi_version.h
Outdated
| @@ -0,0 +1,3 @@ | |||
| #ifndef GPUARRAY_ABI_VERSION | |||
| #define GPUARRAY_ABI_VERSION "1.0" | |||
| #endif | |||
There was a problem hiding this comment.
I think this file should not be commited. Add it in the .gitignore file to help prevent readding it.
|
|
||
| /* The following included file should have been generated by CMake. */ | ||
| #include <gpuarray/abi_version.h> | ||
| #define GPUARRAY_API_VERSION 0 |
There was a problem hiding this comment.
I would remove this line, as it should be in the included file. Otherwise, it is an error and not having it with a wrong value should help with that.
There was a problem hiding this comment.
The included file contains GPUARRAY_ABI_VERSION only, not API version !
to .gitignore.
src/CMakeLists.txt
Outdated
|
|
||
| get_target_property(GPUARRAY_ABI_VERSION gpuarray VERSION) | ||
| FILE(WRITE gpuarray/abi_version.h | ||
| "\#ifndef GPUARRAY_ABI_VERSION\n\#define GPUARRAY_ABI_VERSION \"${GPUARRAY_ABI_VERSION}\"\n\#endif\n" |
There was a problem hiding this comment.
Please make it an int like (major * 1000 + minor). Having it as a string is just super inconvenient for most uses.
.gitignore
Outdated
| *.so | ||
| *.o | ||
| *.log | ||
| src/gpuarray/abi_version.h |
There was a problem hiding this comment.
Group this with the other file in src below.
|
I'm ok with the idea, just don't use strings. |
Change ABI minor version (1.0 -> 1.1) to make some tests.
src/CMakeLists.txt
Outdated
| MACOSX_RPATH OFF | ||
| # This is the shared library version | ||
| VERSION 1.0 | ||
| VERSION 1.1 |
There was a problem hiding this comment.
This doesn't warrant an ABI change. This is an API change.
(equals to the first part of VERSION). Now the built library file is named libgpuarray.so.SOVERSION, that is libgpuarray.so.ABIMAJOR.
|
Update. Add CMake variable SOVERSION (auto-generated and equals to the first part of VERSION, so that VERSION still remains the only variable to update when changing gpuarray ABI version). Now the fix for theano issue Theano/Theano#5345 works ! Theano does not recompile if VERSION changes from 1.X to 1.Y, but recompilation is done if VERSION changes from X.? to Y.? . No more errors in both cases. |
abergeron
left a comment
abergeron
left a comment
There was a problem hiding this comment.
Apart from the version, everything seems fine.
src/CMakeLists.txt
Outdated
| MACOSX_RPATH OFF | ||
| # This is the shared library version | ||
| VERSION 1.0 | ||
| VERSION 2.91 |
There was a problem hiding this comment.
Please reset this to the right version (1.0 for now)
|
Updated. Back to current version (1.0). |
3 participants
Add GPUARRAY_ABI_VERSION and pygpu.gpuarray.abi_version() method to handle ABI version in gpuarray. A new header file (abi_version.h) auto-generated by CMake has been added for this purpose.
These changes must allow to fix Theano issue Theano/Theano#5345 . A related Theano PR is coming.
@abergeron @nouiz