2012-05-02 13 views
0

私はA、B、C、D、E、Fという6つの項目を持つプログラムを書いています。別の配列は、各項目のそれぞれの確率を格納します。与えられた確率に基づいて文字を選択するプログラムの論理エラー

私の仕事は、確率に従ってアイテムを選択する関数を書くことです。すなわち、アイテムAが20%の確率を有する場合、5000回の実行において、時間Aの20%が返されるべきである。

私はこれを行うプログラムを書いていますが、結果の確率が必要な確率と異なるため、論理的な誤りがあるようです。

アルゴリズムに問題がある場合や改善する方法がある場合は、誰かに教えてもらえますか?

#include <iostream> 
#include <array> 
#include <stdlib.h> 
#include <time.h> 

int Probability(std::array<int,6> Probabilities) 
{ 
    //To calculate probability: 
    //Create a new array of size Probabilites. 
    std::array<int,Probabilities.size()> Limits; 

    //At each index of the array store total of all previous probabilites 
    Limits[0] = 0; 

    for(int i=1; i<Limits.size();i++) 
    { 
     Limits[i] = (Limits[i-1] + Probabilities[i]); 
    } 

    //Get a random number. 

    int RandomNumber = rand()%100 + 1; 
    //std::cout<<"Random: "<<RandomNumber<<"\n"; 

    //find between which indexes the random number lies and return the lower one. 
    for(int i=0;i<Limits.size();i++) 
    { 
     if(RandomNumber >= Limits[i] && RandomNumber< Limits[i+1]) 
     { 
      //std::cout<<RandomNumber<<" is between "<<Limits[i]<<"(index="<<i<<") and "<<Limits[i+1]<<"(index="<<i+1<<"). Returning "<<i<<".\n"; 
      return i; 
     }else continue; 
    } 
} 

int main() 
{ 
    const int   SPINS   = 5000; 
    std::array<char,6> Characters = {'A','B','C','D','E','F'}; 
    std::array<int,6> Probabilities = {10,5,2,38,25,20}; 
    std::array<int,6> Frequencies = {0,0,0,0,0,0}; 
    std::srand(time(NULL)); 

    for(int i=0;i<SPINS;i++) 
    { 
     Frequencies[Probability(Probabilities)]++; 
    } 

    int TotalPercentage=0; 
    for(int i=0;i<Frequencies.size();i++) 
    { 
     float Percentage = (Frequencies[i]*100)/5000; 
     TotalPercentage+=Percentage; 
     std::cout<<Characters[i]<<"\t"<<Percentage <<"\t"<<Probabilities[i]<<"\n"; 
    } 

    std::cout<<"Total"<<"\t"<<TotalPercentage<<"\t"<<100<<"\n"; 
} 

結果

アイテム度Prob度Prob(REQ)

B 1 5

C 37 2

D 24 3 8

E 20 25

F 0 20

合計97 100

+2

これは[CodeReview.SE](http://codereview.stackexchange.com/)に属します。 – ildjarn

+2

Rand()%100が間違っていると、低い値にバイアスが追加されます。そして、+1もおそらく望んでいないでしょう。 – wildplasser

+2

あなたはC++ 11をはっきりと使用していますので、C++ 11で導入された非常に優れた乱数生成機能を使用してみませんか?また、なぜあなたは 'のために(...){if(...)else else; } 'else else'は何もしません。 –

答えて

5

ここには4つの大きな問題と2つのマイナーな問題があります。

問題#1:あなたがループの内側から復帰しない場合未定義の動作

for(int i=0;i<Limits.size();i++) 
{ 
    if(RandomNumber >= Limits[i] && RandomNumber< Limits[i+1]) 
    { 
     //std::cout<<RandomNumber<<" is between "<<Limits[i]<<"(index="<<i<<") and "<<Limits[i+1]<<"(index="<<i+1<<"). Returning "<<i<<".\n"; 
     return i; 
    }else continue; 
} 

はどうなりますか?制限を間違って定式化しているため(下記参照)、コードはループの最下部から抜けることがあります。あなたのループの終わりと関数を終わらせる終わりの中括弧の間には、return文もスローもコードもありません。これは未定義の動作です。

何らかの値を返す関数の終わりに近い括弧に達した場合はエラーですが、エラーを通知します(例:throw)。エラーでない場合は、適切な値を返します。空でない関数が閉じ括弧に当たらないようにしてください。

問題#2:Limits

std::array<int,Probabilities.size()> Limits; 

あなたLimitsアレイを使用している方法のための不正なサイズ、あなたはこれがProbabilities.size()+1ようなサイズが必要です。たとえば、{10,5,2,38,25,20}の確率の上限は{0,10,15,17,55,80,100}です。

しかし、あなたのLimits配列にProbabilities配列と同じサイズを与えるより良い方法があります。単に初期ゼロを落とすだけです。

問題#3:Limits

Limits[0] = 0; 
for(int i=1; i<Limits.size();i++) 
{ 
    Limits[i] = (Limits[i-1] + Probabilities[i]); 
} 

誤った内容これは間違ってあなたのLimits配列を移入します。 Probabilities[i]ではなく、Probabilities[i-1]を追加する必要があります。

問題#4:+1あなたLimits[0]と矛盾していることをLimitsRandomNumber

Limits[0] = 0; 
... 
int RandomNumber = rand()%100 + 1; 

矛盾。下位ビットを作る際の問題を抱えているはPRNG

rand()%100 

でも良い乱数発生器からの下位ビットを使用する:Limits[0] 5問題#

1にどちらかが1を追加したり、設定しないでくださいランダム。上位ビットを使用するほうがはるかに優れています。これを行う1つの方法は、ランダムな整数を0と1の間の倍精度に変換し、次にintに変換することです。さらに良い方法は、0と1の間の一様な倍精度を生成するランダムジェネレータを使用することです。さらに良い方法は、バックグラウンドでこれらすべての処理を行うランダムジェネレータを使用することです。

問題#6:rand()

rand() 

rand()を使用すると良い乱数ジェネレータではありません。あなたはC++ 11(C++ 03はstd::arrayを持っていません)を使用しているので、あなたはその言語で提供されている優れた乱数生成器を使用しているはずです。

+0

その答えはそれほど良くはありません。私は新しいランダムライブラリを調べる予定です。あなたは正しいです、私は限界[0] = 0の線で自分自身のためにもっと難しくしました。どうもありがとう :-) –

2

私はあなたの問題は、均一な分布を生成する方法としてrandを使用してから来ていると思います。

は、randの問題についての非常に興味深い記事であり、いくつかのより良い代替案の説明です。

これが役に立ちます。

1

限界を計算するときには、私は、Probabilities[i-1]を使用する必要はありません。

Limits[i] = (Limits[i-1] + Probabilities[i-1]);

それは一種のは明らかだ理由を知るために、あなたがそうでなければ、実際にI = 0で、確率を見ては決してないだろうことに注意してください。

実際には、クリーナーソリューションはを初期化し、上記の行は変更しないでください。あなたのテストは次のようになります:

関連する問題