icl-utk-edu / heffte

BSD 3-Clause "New" or "Revised" License
20 stars 15 forks source link

Make benchmarks, examples, and tests optional #38

Closed streeve closed 10 months ago

streeve commented 10 months ago

New configure options for downstream codes that may not need everything to be built

mkstoyanov commented 10 months ago

Let me try to understand the use-case here.

Right now, if any downstream user employs the CMake add_subdirectory() command on heFFTe, you will get the library but without any of the extra testing or benchmarking.

Using heFFTe externally, the way I see it, means the user builds/tests/installs heFFTe as a one-time cost, then uses it repeatedly. In this case, disabling testing will save little, but will open the door for potential uncaught issues.

I can only see a benefit of skipping testing as part of a downstream CI system. Even then, you will gain only the benefit of shorter compile time, which isn't that much to begin with.

@streeve are you making the PR with CI in mind?

If so, then testing should be ON for the rest of the cases. Also, why the split of examples, benchmarks and tests, only tests take appreciable time to compute.

Later on around line 289:

if (Heffte_ENABLE_TESTING)
    add_subdirectory(benchmarks)
    add_subdirectory(examples)
    enable_testing()
    add_subdirectory(test)
endif()
if (NOT ${CMAKE_PROJECT_NAME} STREQUAL ${PROJECT_NAME})
    if (CUDA_INCLUDE_DIRS)
        # cuda_add_library() apparently adds the includes as private
        target_include_directories(Heffte PUBLIC ${CUDA_INCLUDE_DIRS})
    endif()
    add_library(Heffte::Heffte INTERFACE IMPORTED GLOBAL)
    target_link_libraries(Heffte::Heffte INTERFACE Heffte)
endif()
streeve commented 10 months ago

are you making the PR with CI in mind?

Largely yes and similarly for some docker workflows

Also, why the split of examples, benchmarks and tests, only tests take appreciable time to compute.

Just a habit I suppose. Your suggestions make sense - I'll make the changes