CMAKE: add check for ZFP_CUDA #2937
Conversation
|
Hi I have got this error if my ZFP version is too old to support cuda. But cmake still thinks the ZFP version is okay. Will this PR solve this problem? /home/w4g/d/a2/source/adios2/operator/compress/CompressZFP.cpp: In member function ‘zfp_stream* adios2::core::compress::CompressZFP::GetZFPStream(const Dims&, adios2::DataType, const Params&) const’: |
Yeah this PR attempts to solve this, but now that I see it it needs a bit more work |
vicentebolea
force-pushed
the
add-zfp-cuda-test
branch
from
321bc74 to
d0fbb7f
Compare
|
@JasonRuonanWang could you point me to which version of ZFP are you using? |
| endif() | ||
|
|
||
| if(ADIOS2_HAVE_ZFP_CUDA) | ||
| add_compile_definitions(ADIOS2_HAVE_ZFP_CUDA) |
There was a problem hiding this comment.
@chuckatkins is it ok to add defines like this in ADIOS?
I actually don't know. I had this error and then I realized that it was probably a version issue, and then I just upgraded my ZFP installation and then it worked. So... haven't kept any info of the old installation. Sorry. It should be pretty easy to check the ZFP source code though. Just start from the newest version and check backward, until you see there is no zfp_exec_cuda defined in the enum, then that's the version cmake should say no to. |
vicentebolea
force-pushed
the
add-zfp-cuda-test
branch
from
d0fbb7f to
a75d822
Compare
vicentebolea
force-pushed
the
add-zfp-cuda-test
branch
from
a75d822 to
5e9e7b4
Compare
|
@JasonRuonanWang find many changes here, I tested it for 0.5.1 to 0.5.5 and everything is good, I guess you have to take my word here until we have the CI build to test this. There is also a bit of refactor hereto encapsulate the dependencies a bit better. |
|
@vicentebolea I can live with this. I am fine as long as it compiles and allows me to work on the operators. |
|
@vicentebolea Is this good to go? |
|
@JasonRuonanWang yes |
In the previous PR #2920 which added among other things tests for ZFP with CUDA it does not check if the current ZFP installation supports ZFP.
For this purpose I added a PR at the ZFP which adds a parameter to see this (LLNL/zfp#147).
While this has been merged to keep compatibility to previous commits of ZFP I added some logic at our cmake code to figure out if we have ZFP_CUDA.
EDITED
I made some refactor at the compressZFP so that it moves out the include from the header to the implementation file, also I changed some of the defines provide from ZFP to more stable ones.
I also made the code compatible for ZFP 0.5.1 (the one that we publicly support) to 0.5.5 (latest).