odelaneau / shapeit5

Segmented HAPlotype Estimation and Imputation Tool
https://odelaneau.github.io/shapeit5/
MIT License
61 stars 9 forks source link

random_shuffle() not available in C++17 #6

Closed zyhuang closed 1 year ago

zyhuang commented 1 year ago

Hello again,

I ran into this build error when building phase rare tool.

g++ -std=c++17 -O3 -mavx2 -mfma -D__COMMIT_ID__=\"914aaf4\" -D__COMMIT_DATE__=\"2022-12-16\" -c src/containers/conditioning_set/conditioning_set_solve.cpp -o obj/conditioning_set_solve.o -Isrc -I/install/usr/local/include/htslib -I/install/usr/local/include
src/containers/conditioning_set/conditioning_set_solve.cpp:37:2: error: use of undeclared identifier 'random_shuffle'
        random_shuffle(A.begin(), A.end());
        ^
1 error generated.
make: *** [obj/conditioning_set_solve.o] Error 1

https://github.com/odelaneau/shapeit5/blob/6647bc39866daf8f128991a773ac451e0e00e640/phase_rare/src/containers/conditioning_set/conditioning_set_solve.cpp#L37

and similar error with src/containers/conditioning_set/conditioning_set_selection.cpp. https://github.com/odelaneau/shapeit5/blob/6647bc39866daf8f128991a773ac451e0e00e640/phase_rare/src/containers/conditioning_set/conditioning_set_selection.cpp#L38

It seems the Makefile used C++17 compiler option https://github.com/odelaneau/shapeit5/blob/acda0f5a8765849a1cfe7c11c3715b6a4acc8f11/phase_rare/makefile#L2

while random_shuffle() is removed in C++17 according to cppreference

I got around with the following patch

diff --git a/phase_rare/src/containers/conditioning_set/conditioning_set_selection.cpp b/phase_rare/src/containers/conditioning_set/conditioning_set_selection.cpp
index d92a97b..2adf58c 100644
--- a/phase_rare/src/containers/conditioning_set/conditioning_set_selection.cpp
+++ b/phase_rare/src/containers/conditioning_set/conditioning_set_selection.cpp
@@ -35,7 +35,10 @@ void conditioning_set::select(variant_map & V, genotype_set & G) {
        vector < int > R = vector < int > (n_haplotypes, 0);
        vector < int > M = vector < int > (depth_common * n_haplotypes, -1);
        iota(A.begin(), A.end(), 0);
-       random_shuffle(A.begin(), A.end());
+
+    std::random_device rd;
+    std::mt19937 g(rd());
+    std::shuffle(A.begin(), A.end(), g);

        //Select new sites at which to trigger storage
        vector < vector < int > > candidates = vector < vector < int > > (sites_pbwt_grouping.back() + 1);
diff --git a/phase_rare/src/containers/conditioning_set/conditioning_set_solve.cpp b/phase_rare/src/containers/conditioning_set/conditioning_set_solve.cpp
index 0217232..01684fc 100644
--- a/phase_rare/src/containers/conditioning_set/conditioning_set_solve.cpp
+++ b/phase_rare/src/containers/conditioning_set/conditioning_set_solve.cpp
@@ -34,7 +34,10 @@ void conditioning_set::solve(variant_map & V, genotype_set & G) {
        vector < int > D = vector < int > (n_haplotypes, 0);
        vector < int > R = vector < int > (n_haplotypes, 0);
        iota(A.begin(), A.end(), 0);
-       random_shuffle(A.begin(), A.end());
+
+    std::random_device rd;
+    std::mt19937 g(rd());
+    std::shuffle(A.begin(), A.end(), g);

        //Get cM positions of the scaffold sites
        vector < float > vs_cm = vector < float > (n_scaffold_variants, 0.0);

The git version is v1.0.0-beta-20-gacda0f5.

Please suggest if there are alternative ways to fix this. Thanks!

Best, Zhuoyi

LouisLeNezet commented 1 year ago

I was confronted to the same issue while compiling for bioconda. So I went with a -std=c++14 compiler. @odelaneau or @srubinacci do you have a better way of doing it ?

odelaneau commented 1 year ago

Thanks, I added zyhuang's fix based on shuffle. Cheers, O