配列の中央値を求めるプログラムで毎回0しか出力されない問題について


繰り返し処理の各処理におけるパフォーマンスの中央値を求めるプログラムを書いています。  
以下の記事を参考にして、配列から中央値を求める部分を書きました。  
[Vectorの平均値と中央値を求める](https://qiita.com/kajitack/items/aaf4721c55e372de3c20)

コンパイルエラーはなく、実行時の出力において、中央値の箇所が毎回「０」になってしまうという問題があります。  
どのように、プログラムを修正すべきか検討がつかず、教えていただきたいです。

出力

[code]

    173, 
    85, 
    85, 
    85, 
    After Sorted
    85, 
    85, 
    85, 
    173, 
    0
    
[/code]

現行のコード

[code]

    #include <chrono>
    #include <iostream>
    #include <thread>
    #include <vector>
    #include <algorithm>
    #include <iostream>
    
    //繰り返し処理全体にかかった時間
    using namespace std::chrono;
    
    double median(std::vector<double>v) {
        size_t size = v.size();
        std::vector<double> _v(v.size());
        copy(v.begin(), v.end(), back_inserter(_v));
        double tmp;
        for (int i = 0; i < size - 1; i++){
            for (int j = i + 1; j < size; j++) {
                if (_v[i] > _v[j]){
                    tmp = _v[i];
                    _v[i] = _v[j];
                    _v[j] = tmp;
                }
            }
        }
        if (size % 2 == 1) {
            return _v[(size - 1) / 2];
        } else {
            return (_v[(size / 2) - 1] + _v[size / 2]) / 2;
        }
    }
    
    int main()
    {
        #define N 2
    
        double A[N][N] = {
            {3.0, 5.0},
            {9.0, 5.0}
        };
    
        double B[N][N] = {
            {3.0, 6.0},
            {8.0, 9.0}
        };
    
        double C[N][N] = {
            {0.0, 0.0},
            {0.0, 0.0}
        };
        int i, j, k, n;
    
    
    
        //各足し算にかかった時間を入れる配列
        std::vector<double> timedata;
    
        for(i=0; i<N; i++){
            for(j=0; j<N; j++){
                    //測定開始
                    std::chrono::high_resolution_clock::time_point start = std::chrono::high_resolution_clock::now();
                    C[i][j] += A[i][j]+B[i][j];
                    //測定終了
                    high_resolution_clock::time_point end = high_resolution_clock::now();
                    double time =  std::chrono::duration_cast<std::chrono::nanoseconds>(end - start).count();
    
                    //配列にappend
                    timedata.push_back(time);
                }
        }
    
        //ソート前
        for (auto value : timedata) {std::cout << value << ", ";
            std::cout << std::endl;
        }
    
        //配列をソート
        std::cout << "After Sorted"<< std::endl;
        std::sort(timedata.begin(), timedata.end());
        for (auto value : timedata) {std::cout << value << ", ";
            std::cout << std::endl;
        }
    
        double ans = median(timedata);
        std::cout << ans << std::endl;
    
    }
    
    
    
[/code]




直接の原因は`median()`にあります。

[code]

    double median(std::vector<double>v) {
        size_t size = v.size();
        std::vector<double> _v(v.size());            // (1)
        copy(v.begin(), v.end(), back_inserter(_v)); // (2)
    
[/code]

(1)の行では、引数の`v`と同じ個数の要素を持つ別のvector
`_v`を作成しています。例えば`v`のサイズが3だとすれば、`_v`も3つの要素を持っています。  
(2)の行で、`v`の中身を`_v`にコピーしていますが、ここで`back_inserter`を使っているため、すでにある`_v`の中身に`v`の中身を追加する形でコピーしています。そのため、この後の`v`の中央値を求めるアルゴリズムが全く的外れになっています。

そもそも、引数が`std::vector<double>v`なので、関数が呼び出された時点で、`v`の中身はオリジナルではなくコピーです。なのでコピーを自分でするコードがそもそも余計です。

元のコードをなるべく活かす形で修正するなら

[code]

    double median(std::vector<double>v) {
        size_t size = v.size();
        double tmp;
        for (int i = 0; i < size - 1; i++) {
            for (int j = i + 1; j < size; j++) {
                if (v[i] > v[j]) {
                    tmp = v[i];
                    v[i] = v[j];
                    v[j] = tmp;
                }
            }
        }
        if (size % 2 == 1) {
            return v[(size - 1) / 2];
        }
        else {
            return (v[(size / 2) - 1] + v[size / 2]) / 2;
        }
    }
    
[/code]

でしょうか。

そもそも`median`を呼び出す前に配列をソートしているのに、`median`内でソートしなおしているなど、他にも変なところはありますが、ここでは言及しません。

* * *

老婆心ながら、どこかのコードを、何をしているかを理解もせず、コピペして使うのはやめましょう。正しい保証はありませんし、勉強にもなりません。




直接の原因は`median()`にあります。

[code]

    double median(std::vector<double>v) {
        size_t size = v.size();
        std::vector<double> _v(v.size());            // (1)
        copy(v.begin(), v.end(), back_inserter(_v)); // (2)
    
[/code]

(1)の行では、引数の`v`と同じ個数の要素を持つ別のvector
`_v`を作成しています。例えば`v`のサイズが3だとすれば、`_v`も3つの要素を持っています。  
(2)の行で、`v`の中身を`_v`にコピーしていますが、ここで`back_inserter`を使っているため、すでにある`_v`の中身に`v`の中身を追加する形でコピーしています。そのため、この後の`v`の中央値を求めるアルゴリズムが全く的外れになっています。

そもそも、引数が`std::vector<double>v`なので、関数が呼び出された時点で、`v`の中身はオリジナルではなくコピーです。なのでコピーを自分でするコードがそもそも余計です。

元のコードをなるべく活かす形で修正するなら

[code]

    double median(std::vector<double>v) {
        size_t size = v.size();
        double tmp;
        for (int i = 0; i < size - 1; i++) {
            for (int j = i + 1; j < size; j++) {
                if (v[i] > v[j]) {
                    tmp = v[i];
                    v[i] = v[j];
                    v[j] = tmp;
                }
            }
        }
        if (size % 2 == 1) {
            return v[(size - 1) / 2];
        }
        else {
            return (v[(size / 2) - 1] + v[size / 2]) / 2;
        }
    }
    
[/code]

でしょうか。

そもそも`median`を呼び出す前に配列をソートしているのに、`median`内でソートしなおしているなど、他にも変なところはありますが、ここでは言及しません。

* * *

老婆心ながら、どこかのコードを、何をしているかを理解もせず、コピペして使うのはやめましょう。正しい保証はありませんし、勉強にもなりません。

