57085


繰り返し処理の各処理におけるパフォーマンスの中央値を求めるプログラムを書いています。  
以下の記事を参考にして、配列から中央値を求める部分を書きました。  
[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]


CC BY-SA 4.0
2019-07-31T14:17:48.637
57082
2019-08-01T01:35:28.570
2019-07-31T14:52:04.380
question
c++ アルゴリズム
配列の中央値を求めるプログラムで毎回0しか出力されない問題について

[code]

    std::vector<double> _v(v.size());
    copy(v.begin(), v.end(), back_inserter(_v));
    
[/code]

この部分でおそらく `_v` に `v` を丸々コピーしようとしてるのでしょうが、うまくいってません。  
最初の行で `v.size()` 分の要素をデフォルト値(0.0)で埋めた後、次の行でその後ろに v の要素を追加してます。  
このコードを呼び出した後の `_v` の状態が想定したものと同じか確認してください。


CC BY-SA 4.0
2019-07-31T15:59:22.757
57084
2019-07-31T15:59:22.757
33033
57082
answer


直接の原因は`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`内でソートしなおしているなど、他にも変なところはありますが、ここでは言及しません。

* * *

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


CC BY-SA 4.0
2019-07-31T16:12:43.853
57085
2019-07-31T16:12:43.853
3605
57082
answer


質問タイトルはまるっきり無視して

ソート済み `vector` が既にあるのだから中央値を求める手続きは
[vectorに関するコンパイルエラー](https://ja.stackoverflow.com/questions/57054/)
のコードでほぼ問題ないっす。 `vector` の要素数を得るには `sizeof` ではなくて `v.size()` であることだけ修正。

[code]

    std::sort(timedata.begin(), timedata.end());
    size_t n=timedata.size();
    // n==0 のとき値が無いわけだが、今回は考慮しない
    if ((n%2)==0) med=(timedata[(n/2)-1]+timedata[n/2])/2.0;
    else med=timedata[n/2];
    
[/code]

[c++](/questions/tagged/c%2b%2b "'c++' のタグが付いた質問を表示") 本職さん相手なら、コンテナが
`std::list` でも問題ないようにするには `std::advance` を使えとかいう話に展開していくんだけど今回は略。

* * *

鍵括弧の使い方ちゃんと認識してます？ 必要なところにつけて必要のないところで略す主義と、常にいつもつける主義とあります。  
オイラのサンプル
[C++で生成した配列要素のがソートできず、中身の要素数が不自然な問題について](https://ja.stackoverflow.com/questions/57071/)
は前者なので括弧を書いていないのはわざとっす。今回のソースコードのように `{ ... }` 内部に `std::cout << std::endl;`
を入れちゃうと意味も挙動も違ってくるっすよ。

[code]

    // １行に複数個、カンマで区切って出力
    for (auto value : timedata) std::cout << value << ", ";
    // 次に何か出力したいとき、混ざらないようループ後に改行出力
    std::cout << std::endl;
    
[/code]

コメントに書いたようなことを認識しているなら括弧はこういうふうにつけるはず

[code]

    for (auto value : timedata) {
        std::cout << value << ", ";
    }
    std::cout << std::endl;
    
[/code]


CC BY-SA 4.0
2019-08-01T01:35:28.570
57089
2019-08-01T01:35:28.570
8589
57082
answer