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.

Sign up for GitHub

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

Jump to bottom

Allow debug builds with cmake -DCMAKE_BUILD_TYPE=Debug #128

Closed
pknowles opened this issue Aug 15, 2023 · 6 comments
Closed

Allow debug builds with cmake -DCMAKE_BUILD_TYPE=Debug #128

pknowles opened this issue Aug 15, 2023 · 6 comments

Comments

@pknowles
Copy link

It would be good to avoid the following, so that cmake can be configured for debug builds:

set(CMAKE_BUILD_TYPE "Release" CACHE STRING "" FORCE)

See: https://stackoverflow.com/a/23916503/1888983
@DTolm
Copy link
Owner

DTolm commented Aug 15, 2023

Hello.

The CMake is used to configure the benchmark/precision verification suite. Why would you not want it to be built in Release? The library itself is header-only, so this should not affect the actual usage of it.

Best regards,
Dmitrii

@al42and
Copy link
Contributor

al42and commented Aug 17, 2023

Why would you not want it to be built in Release?

Not sure what's @pknowles's case, but I used the debug build for solving #124. The flow was like: "Our application now crashes. But is it a bug in VkFFT itself or in how we use it? Let's try the benchmark suite. Crashes too? Ok, now we know that it's not our code. Let's run the benchmark suite in debugger and look at the stack trace to avoid any extra noise and abstraction layers." (in the end, there was an unrelated issue with our code, but that's another story)

@DTolm
Copy link
Owner

DTolm commented Aug 17, 2023

Ok, but that is one line of code that is needed to change in CMakeLists. The main purpose of the benchmark is to measure the best performance, so it makes sense to force the Release build for this - I think I originally enabled it as there were some performance drops in the cuFFT library and I wanted the comparison to be fair.

@al42and
Copy link
Contributor

al42and commented Aug 17, 2023

Ok, but that is one line of code that is needed to change in CMakeLists. The main purpose of the benchmark is to measure the best performance, so it makes sense to force the Release build for this

Can't argue here.

But it is also not hard to surround this line with if(NOT CMAKE_BUILD_TYPE)/endif() to allow the users to change the build type with normal CMake means if they so wish, while still using Release by default.

@DTolm
Copy link
Owner

DTolm commented Aug 17, 2023

True, I can add this in the next update. Thanks!

DTolm added a commit that referenced this issue Sep 25, 2023
@DTolm
Quad precision (double-double) support in VkFFT
ae5a334 
-Added double-double support in VkFFT. Requires cpu initialization in full quad precision, so only supports gcc for now. Potentially possible to add full FP128 support or some other FP128 library (like mpir) in the future.
-Data has to be stored in double-double before VkFFT kernels calls (no fp128<->double-double conversion on the GPU yet).
-Full 1e-32 precision, but same range as FP64. See Library for Double-Double and Quad-Double Arithmetic by Y Hida for more information on double-double.
-Reuqires FMA contraction to be disabled (due to ab-cd contraction rounding mismatch). Doesn't work on Vulkan as I haven't found how to do that yet.
-Fixed warnings (#138)
-Added proper check for app to be zero before initializeVkFFT call and zeroing on deletion (#134)
-Added an option to provide staging buffer in application and VkGPU handle (#129)
-Added guards for build type (#128)
-Fixed missing deallocation calls for the inverse Bluestein axes. Fixed the buffer layout size in Vulkan in some cases.
-Refactored the code generator and container struct layout for better handling complex numbers (-5k loc).
-Added more precision tests and benchmarks.
-Will be merged in the main branch after more testing and update to the documentation.
@DTolm
Copy link
Owner

DTolm commented Sep 25, 2023

Dear All,

These guards have been added to the 1.3.2 version.

Best regards,
Dmitrii

@DTolm DTolm mentioned this issue Oct 23, 2023
Merged
DTolm added a commit that referenced this issue Oct 23, 2023
@DTolm
VkFFT v1.3.2 release
3fa0c21 
-Added double-double support in VkFFT. Requires cpu initialization in full quad precision, so only supports gcc with quadmath dependency for now. Potentially possible to add full FP128 support or some other FP128 library (like mpir) in the future.
-Data has to be stored in double-double before VkFFT kernels calls (no fp128<->double-double conversion on the GPU yet).
-Full 1e-32 precision, but same range as FP64. See Library for Double-Double and Quad-Double Arithmetic by Y Hida for more information on double-double.
-Double-double requires FMA contraction to be disabled (due to ab-cd contraction rounding mismatch). Doesn't work on Vulkan as I haven't found how to do that yet.
-Added DST I-IV support.
-Fixed warnings (#138)
-Added proper check for app to be zero before initializeVkFFT call and zeroing on deletion (#134)
-Added an option to provide a staging buffer in the application and VkGPU handle (#129)
-Added guards for build type (#128)
-Changed default innermost stride for real buffers in out-of-place R2C from size[0]+2 to size[0] (#139)
-Allow specifying glslang version (#135)
-Improved instruction count and accuracy for radix-7.
-Fixed missing deallocation calls for the inverse Bluestein axes. Fixed the buffer layout size in Vulkan in some cases.
-Refactored the code generator and container struct layout for better handling complex numbers (-5k loc).
-Added more precision tests and benchmarks.
@DTolm DTolm closed this as completed Oct 23, 2023
@DTolm DTolm mentioned this issue Nov 7, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@al42and @pknowles @DTolm