Adding cmake build & test

[DiamonDinoia]

Dear all,

I started working on implementing the CI using cmake.
Following steps:

• Fix Ubuntu GCC tests
• Add Ubuntu Clang build OpenMP not found egor-tensin/setup-clang#12
• Fix Windows build. Tests use unistd.h
• Add Windows msvc build & tests
• Add Windows clang build & tests
• Fix mac-os build (OpenMP missing)
• Add mac-os build & tests

[DiamonDinoia]

@lu1and10, do you have experience with github workflows?
If you could help me install clang on linux and windows, msvc on windows and clang with openmp on mac we can automate this.
We can also automate artifact generation. Especially on windows, compiling with avx generates not vector code hence the code is portable.

[lu1and10]

@lu1and10, do you have experience with github workflows?
If you could help me install clang on linux and windows, msvc on windows and clang with openmp on mac we can automate this.
We can also automate artifact generation. Especially on windows, compiling with avx generates not vector code hence the code is portable.

@DiamonDinoia I used clang with openmp on mac with github workflows. Never used clang on linux and windows with github workflows. I can look into these two setups. Not sure about msvc, I guess. msvc needs license to use?

[DiamonDinoia]

@lu1and10, do you have experience with github workflows?
If you could help me install clang on linux and windows, msvc on windows and clang with openmp on mac we can automate this.
We can also automate artifact generation. Especially on windows, compiling with avx generates not vector code hence the code is portable.

@DiamonDinoia I used clang with openmp on mac with github workflows. Never used clang on linux and windows with github workflows. I can look into these two setups. Not sure about msvc, I guess. msvc needs license to use?

No, MSVC is free to use. There are licenses but that is for visual studio premium features.

[ahbarnett]

You'll see in #288 I made ctest test everything that the classic makefile did (barring setting OMP_NUM_THREADS to something small in case you're on a massive CPU node). This means CI is ready to switch over to ctest. Mostly pinging @lu1and10 on this.

[DiamonDinoia]

we can set the number of threads in software:

#include <omp.h>
#include <thread>
#include <algorithm>

constexpr max_threads = k;

void set_threads() {
    const auto processor_count = std::thread::hardware_concurrency();
    const auto threads = std::min(max_threads, processor_count);
    omp_set_num_threads(threads);
}

[ahbarnett]

Sure, we already do this. (opts.nthreads; eg I use it in dumbinputs.cpp). But the finufft?n{many}_test executables don't control this via the command line args. To do so I'd have to rewrite 6 sets of args processors; I'd rather not. Hence the desire for the env var control...

…

On Sat, Jun 17, 2023 at 5:58 AM Marco Barbone ***@***.***> wrote: we can set the number of threads in software: #include<omp.h> #include <thread> #include <algorithm> constexpr max_threads = k; void set_threads() { const auto processor_count = std::thread::hardware_concurrency(); const auto threads = std::min(max_threads, processor_count); omp_set_num_threads(threads); } — Reply to this email directly, view it on GitHub <#254 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACNZRSTE465NGAPXGDT3LBTXLV5VHANCNFSM6AAAAAAV2HPS3A> . You are receiving this because you commented.Message ID: ***@***.***>

-- *-------------------------------------------------------------------~^`^~._.~' |\ Alex Barnett Center for Computational Mathematics, Flatiron Institute | \ http://users.flatironinstitute.org/~ahb 646-876-5942

[DiamonDinoia]

You could use add custom command to specify environment variables and use it as a custom target. Then pass the target to add_test. An example is below. I could refine it on Monday

 add_custom_command(OUTPUT ${DATA}
          COMMAND ${CMAKE_COMMAND} -E env G4LEDATA=${GEANT4_INSTALL_DATADIR}/G4EMLOW8.0
         G4ENSDFSTATEDATA=${GEANT4_INSTALL_DATADIR}/G4ENSDFSTATE2.3
          $<TARGET_FILE:dpm_GenerateData> -c 1 -d ${DATA_DIR} -i ${PROJECT_SOURCE_DIR}/lib/dpm-g4cpp/SBTables/data
            DEPENDS dpm_GenerateData)

add_custom_target(GenerateData SOURCES ${DATA}) 

[DiamonDinoia]

I'm ticking the first task as completed. The test is built correctly but fails so we might want to open a new issue to fix the test itself, not CI: https://github.com/DiamonDinoia/finufft/actions/runs/5310537696/jobs/9612643024#step:4:2143

[lu1and10]

I'm ticking the first task as completed. The test is built correctly but fails so we might want to open a new issue to fix the test itself, not CI: https://github.com/DiamonDinoia/finufft/actions/runs/5310537696/jobs/9612643024#step:4:2143

I try to fix the dumbinputs test and memory leak in the latest master branch commit, it may fix this issue.

[DiamonDinoia]

I try to fix the dumbinputs test and memory leak in the latest master branch commit, it may fix this issue.

It seems fixed: https://github.com/DiamonDinoia/finufft/actions/runs/5314173855/jobs/9621066673

[DiamonDinoia]

#254 (comment)

Hi Alex, with the code I provided below the number of processors is queried at runtime in c++ (that's equivalent to calling nproc in bash). The number of threads is limited only if it is more than k the maximum that we want to allow. I believe this is much simpler than using and environment variable in cmake

we can set the number of threads in software:

#include <omp.h>
#include <thread>
#include <algorithm>

constexpr max_threads = k;

void set_threads() {
    const auto processor_count = std::thread::hardware_concurrency();
    const auto threads = std::min(max_threads, processor_count);
    omp_set_num_threads(threads);
}

[ahbarnett]

Libin has completed most of your requests here on #382. I think we'll bring that in in the next day or two, which would supercede this PR. What we need your input on is to check Libin's use of FINUFFT_CDECL, FINUFFT_EXPORT, and other new macros in the headers, which enable Windows MSVC and Clang users to create dynamic-linked shared library. (as opposed to static).

[DiamonDinoia]

PR #382 is forked from this. We could merge #382 here, mark the checklist and merge into master. This way the comment history will look cleaner?

[lu1and10]

PR #382 is forked from this. We could merge #382 here, mark the checklist and merge into master. This way the comment history will look cleaner?

The original design of the ci file structure, i.e, each OS-Compiler for one *yml is with a lot of replication. Single yml file should suffice, see working example, https://github.com/lu1and10/finufft/blob/cmake-build/.github/workflows/cmake_ci.yml
maybe we should just make a new PR remove all the redundant files history.

[DiamonDinoia]

I think we can just squash the commits? https://gist.github.com/lpranam/4ae996b0a4bc37448dc80356efbca7fa
I am a fan of squashing the commits before opening a PR always. Makes reverting the commits much easier. Unless there is a meaningful git history in the PL.