As the title says I have a out of range error and I can't seem to find it. I added a little documentation to the code, and a debug message and it seems to stop at random places ... its kinda strange.
main.cpp
[cpp]
#include "chromosone.h"
void Reproduce(chromosone &Parrent1,chromosone &Parrent2);
int RouletteSelect(float total_fit, std::vector<chromosone*> population);
int main()
{
float total_fit=0.f;
int find;
bool found=false;
srand((unsigned)time(0));
std::vector<chromosone*> population,temp_pop;
for(unsigned int i=0;i < NUM_CHROMOSONES;i++) // Make the first population
{
chromosone *chrom = new chromosone;
population.push_back(chrom);
}
for(unsigned int i=0;i < MAX_GEN;i++) // For the maximum generation
{
if(!found) // If the sollution is not found.
{
for(unsigned int i=0;i < NUM_CHROMOSONES;i++) // For each chromosone Calculate the fitnes
{
population.at(i)->BinaryToDecimal();
population.at(i)->Decode();
population.at(i)->Parse();
population.at(i)->CalcFit();
if(population.at(i)->fitnes != 1000.f)
{
total_fit+=population.at(i)->fitnes; // Calculate the maximum fitnes
} else { // Answer is found
find = i;
found = true;
break;
}
}
for(unsigned int i=0;i < NUM_CHROMOSONES;i+=2) // For each 2 chromosone's cross them over.
{
std::cout << i << std::endl;
chromosone *p1,*p2;
p1 = population.at(RouletteSelect(total_fit,population));
p2 = population.at(RouletteSelect(total_fit,population));
Reproduce(*p1,*p2);
p1->Mutate();
p2->Mutate();
temp_pop.push_back(p1);
temp_pop.push_back(p2);
}
for(unsigned int i=0;i < NUM_CHROMOSONES-1;i++) // assign the new pop to the old pop
{
population.at(i) = temp_pop.at(i);
}
total_fit = 0.f; // Clear all.
temp_pop.clear();
} else {
break;
}
}
std::cout << "Can't find the sollution !";
}
int RouletteSelect(float total_fit, std::vector<chromosone*> population) // Makes a roullete selection.
{
float random = (float)(RANDOM*total_fit);
float temp_fit = 0.f;
for(unsigned int i=0;i < NUM_CHROMOSONES;i++)
{
temp_fit+=population.at(i)->fitnes;
if(temp_fit > random)
{
return i;
}
}
}
void Reproduce(chromosone &Parrent1,chromosone &Parrent2) // Crosses 2 parrents
{
if(RANDOM < CROSSOVER_RATE)
{
int crossover_point = (int)RANDOM*CHROMO_LENGTH;
Parrent1.bits = Parrent1.bits.substr(0,crossover_point) + Parrent2.bits.substr(crossover_point,CHROMO_LENGTH);
Parrent2.bits = Parrent2.bits.substr(0,crossover_point) + Parrent1.bits.substr(crossover_point,CHROMO_LENGTH);
}
}
[/cpp]
chromosone.h
[cpp]
#include <string>
#include <vector>
#include <sstream>
#include <iostream>
#include <math.h>
#include <ctime>
#define RANDOM ((float)rand()/(RAND_MAX+1))
const double CROSSOVER_RATE=0.7;
const double MUTATION_RATE=0.001;
const int NUM_CHROMOSONES=50;
const int MAX_GEN = 500;
const int CHROMO_LENGTH=200;
const int GENE_LENGTH=4;
const int TO_FIND=103;
class chromosone
{
private:
std::vector<int> answer,Decimal;
enum
{
ADD = 10,
SUB,
MUL,
DIV
};
public:
std::string bits;
float fitnes,fawnser;
void GenerateRandomSequence(); // Generates a random sequence of bits
void Mutate(); // Mutates the gene by flipping bits
void BinaryToDecimal(); // Converts binary to decimal
void Decode(); // Decode's the gene so the parsers can interpret it
void Parse(); // Calculates the answer
void CalcFit(); // Calcs the fitnes
//chromosone(std::string str){fawnser=0;bits = str;}
chromosone(){fawnser=0;GenerateRandomSequence();}
};
[/cpp]
chromosone.cpp
[cpp]
#include "chromosone.h"
void chromosone::GenerateRandomSequence()
{
std::stringstream Stream;
Stream.str(bits);
for(int i=0;i < CHROMO_LENGTH;i++)
{
Stream << rand()%2;
}
bits = Stream.str();
}
void chromosone::Mutate()
{
for(unsigned int i=0;i < bits.size();i++)
{
if(RANDOM < MUTATION_RATE)
{
if(bits.at(i) == '1')
{
bits.at(i) = '0';
} else {
bits.at(i) = '1';
}
}
}
}
void chromosone::BinaryToDecimal()
{
std::string Buffstr; // Bufferstring
int buff=0; // next number to push back
for(int i=0;i < (CHROMO_LENGTH/GENE_LENGTH);i++) // for each gene
{
Buffstr = bits.substr(i*GENE_LENGTH,GENE_LENGTH); // take the bit whe need
for(int k=0;k < GENE_LENGTH;k++)
{
buff = buff*2+atoi(Buffstr.substr(k,1).c_str()); // double the buffer and add the next number in the string.
}
Decimal.push_back(buff); // push back the number
buff = 0; // reset the number.
}
}
void chromosone::Decode()
{
answer.clear();
bool look_for_opp = false;
for(unsigned int i=0; i < Decimal.size();i++)
{
if(look_for_opp)
{
if(Decimal.at(i) > 9 && Decimal.at(i) < 14)
{
answer.push_back(Decimal.at(i));
look_for_opp = false;
}
continue;
}
if(Decimal.at(i) >= 0 && Decimal.at(i) < 10)
{
answer.push_back(Decimal.at(i));
look_for_opp = true;
}
}
}
void chromosone::Parse()
{
//std::cout << fawnser;
for(unsigned int i=0; i+2 < answer.size() ;i+=2)
{
if(answer.at(i+2) >= 0 && answer.at(i+2) < 10)
{
switch(answer.at(i+1)) // Get the operator.
{
case ADD:
//std::cout << " + " << answer.at(i+2);
fawnser+=answer.at(i+2);
break;
case SUB:
//std::cout << " - " << answer.at(i+2);
fawnser-=answer.at(i+2);
break;
case MUL:
//std::cout << " * " << answer.at(i+2);
fawnser*=answer.at(i+2);
break;
case DIV:
if(answer.at(i+2) != 0)
{
//std::cout << " / " << answer.at(i+2);
fawnser/=answer.at(i+2);
}
break;
}
}
}
//std::cout << std::endl;
}
void chromosone::CalcFit()
{
if(fawnser == TO_FIND)
{
fitnes = 1000.f;
} else {
fitnes = 1/(float)fabs((double)(TO_FIND-fawnser));
}
}
[/cpp]
Have you tried using a debugger to step through and see where the error is being thrown?
Yeah. std::vector::at() is what throws it from a quick glance at a code, because operator[] doesn't throw, so check your indexes.
I checked with the debugger and this is what I got :
[img]http://www.cubeupload.com/files/b8e00gen.jpg[/img]
The numbers in the console are the return of the roulette selection
What does the Output-window say? I spot some Run-Time Check error-message there.
Just check the call stack when the exception is thrown?
You can change the frame via the call stack window and view the locals of different calls.
[QUOTE=ZeekyHBomb;19741653]What does the Output-window say? I spot some Run-Time Check error-message there.[/QUOTE]
[code]
Run-Time Check Failure #0 - The value of ESP was not properly saved across a function call. This is usually a result of calling a function declared with one calling convention with a function pointer declared with a different calling convention.
[/code]
But this is because I was stepping out and checking where the error began. Also I opened the call stack window but I'm no expert at debugging code and well I don't know what to do with it.
Wanna screenshot it? You may need to load debug symbols. Cause you should be able to figure the call stack out as it just a list of functions, nothing confusing there...
[img]http://www.cubeupload.com/files/2d600naamloos.jpg[/img]
Here you go.
Double click the "main" function. It is the 4th row.
That will switch the frame and it will show the line that caused the exception and the local variables.
[cpp] for(unsigned int i=0;i < NUM_CHROMOSONES;i+=2) // For each 2 chromosone's cross them over.
{
std::cout << i << std::endl;
chromosone *p1,*p2;
p1 = population.at(RouletteSelect(total_fit,population));
p2 = population.at(RouletteSelect(total_fit,population));
Reproduce(*p1,*p2);
p1->Mutate();
p2->Mutate();
temp_pop.push_back(p1);
temp_pop.push_back(p2);
}
//temp_pop.size() == NUM_CHROMOSOMES/2
for(unsigned int i=0;i < NUM_CHROMOSONES-1;i++) // assign the new pop to the old pop
{
population.at(i) = temp_pop.at(i); //upon i > NUM_CHROMOSOMES/2.. oh noez!
} [/cpp]
Solution: use the size-member of the vectors you are accessing, not NUM_CHROMOSOMES.
[QUOTE=ZeekyHBomb;19743447][cpp] for(unsigned int i=0;i < NUM_CHROMOSONES;i+=2) // For each 2 chromosone's cross them over.
{
std::cout << i << std::endl;
chromosone *p1,*p2;
p1 = population.at(RouletteSelect(total_fit,population));
p2 = population.at(RouletteSelect(total_fit,population));
Reproduce(*p1,*p2);
p1->Mutate();
p2->Mutate();
temp_pop.push_back(p1);
temp_pop.push_back(p2);
}
//temp_pop.size() == NUM_CHROMOSOMES/2
for(unsigned int i=0;i < NUM_CHROMOSONES-1;i++) // assign the new pop to the old pop
{
population.at(i) = temp_pop.at(i); //upon i > NUM_CHROMOSOMES/2.. oh noez!
} [/cpp]
Solution: use the size-member of the vectors you are accessing, not NUM_CHROMOSOMES.[/QUOTE]
Uhm temp_pop consists of 50 chromosone's
It reproduce's 25 times and each reproduce gives 2 children.
I also notice'd that the error is random, somethimes it completed 1-2 maybe 3 generations before it happened.
[PHP]int RouletteSelect(float total_fit, std::vector<chromosone*> population)[/PHP]Doesn't always return a value - something causes the for loop's condition to not always pass for a given input. The missing return value messes up ESP, and causes the error you received, and other, stranger bugs.
[B]Edit:[/B] I also just noticed, that you pass a std::vector. This copies the entire vector, which can be very slow. You should instead pass a const std::vector<chromosone*>&
I can't test it right now but I should have probably done :
if(temp_fit >= random)
Well, that failed, And I don't know how, how can it be that the function doesn't return anything. The maximum fitnes * a number equal or less than 1 can never be higher than the total fit.
Anyone else has some suggestions ?
[cpp]
int RouletteSelect(float total_fit, std::vector<chromosone*> population)
{
float random = (float)(RANDOM*total_fit);
float temp_fit = 0.f;
for(unsigned int i=0;i < NUM_CHROMOSONES;i++)
{
temp_fit+=population.at(i)->fitnes;
if(temp_fit > random)
{
return i;
}
}
assert(0); //Try this
}
[/cpp]
[QUOTE=jA_cOp;19803477][cpp]
int RouletteSelect(float total_fit, std::vector<chromosone*> population)
{
float random = (float)(RANDOM*total_fit);
float temp_fit = 0.f;
for(unsigned int i=0;i < NUM_CHROMOSONES;i++)
{
temp_fit+=population.at(i)->fitnes;
if(temp_fit > random)
{
return i;
}
}
assert(0); //Try this
}
[/cpp][/QUOTE]
[img]http://www.cubeupload.com/files/909800naamloos.jpg[/img]
For debugging I added some debug messages in the console the first number is the generated random and the next is the maximal fitnes.
It's extremely difficult to look through your code as it has quite ambiguous structure and uses a lot of constants and macros.
But here's a little advice on how you could improve the maintainability of your code, and help you debug it:
[cpp]
//You should consider passing this vector by const reference. And what the crap is total_fit supposed to mean?
int RouletteSelect(float total_fit, std::vector<chromosone*> population)
{
//the RANDOM macro is hardly descriptive. Replace it with an inline function that takes arguments, that way you don't have to look elsewhere to see what is actually going on.
float random = (float)(RANDOM*total_fit);
//This function entirely depends on RANDOM doing a specific thing and that NUM_CHROMOSONES is actually less than or equal to the size of the vector. Use the .size method to remove this dependency, and see above for 'RANDOM'
for(unsigned int i=0;i < NUM_CHROMOSONES;i++)
{
//Just what is 'fitnes' supposed to mean? I know where it is set, but it took me a while to find. You should move all initialization code into a constructor: that way you know it's initialized, and you know where.
temp_fit+=population.at(i)->fitnes;
if(temp_fit > random) //Why is it guaranteed that temp_fit will get bigger than random?
{
return i;
}
}
[/cpp]
Your program is extremely hard to read because everything is stuffed into main and you have a lot of unused variables and redundant code (examples; your 'find' variable, and lots of C style static casts you don't really need).
edit:
The cpp tags totally raped my comment structure, but every comment refers to the line of code directly below it.
[QUOTE=jA_cOp;19821714]It's extremely difficult to look through your code as it has quite ambiguous structure and uses a lot of constants and macros.
But here's a little advice on how you could improve the maintainability of your code, and help you debug it:
[cpp]
//You should consider passing this vector by const reference. And what the crap is total_fit supposed to mean?
int RouletteSelect(float total_fit, std::vector<chromosone*> population)
{
//the RANDOM macro is hardly descriptive. Replace it with an inline function that takes arguments, that way you don't have to look elsewhere to see what is actually going on.
float random = (float)(RANDOM*total_fit);
//This function entirely depends on RANDOM doing a specific thing and that NUM_CHROMOSONES is actually less than or equal to the size of the vector. Use the .size method to remove this dependency, and see above for 'MACRO'
for(unsigned int i=0;i < NUM_CHROMOSONES;i++)
{
//Just what is 'fitnes' supposed to mean? I know where it is set, but it took me a while to find. You should move all initialization code into a constructor: that way you know it's initialized, and you know where.
temp_fit+=population.at(i)->fitnes;
if(temp_fit > random) //Why is it guaranteed that temp_fit will get bigger than random?
{
return i;
}
}
[/cpp]
Your program is extremely hard to read because everything is stuffed into main and you have a lot of unused variables and redundant code (examples; your 'find' variable, and lots of C style static casts you don't really need).
edit:
The cpp tags totally raped my comment structure, but every comment refers to the line of code directly below it.[/QUOTE]
Well the 'find' crap is because I threw this together quickly to see if it works, but whats wrong with having constants, this way I can easily change all the parameters when needed. Random generates a number between 0-1, total_fit is the total fitnes of all the chromosome's. The fitnes is a number at how good the chromosome is at solving the problem, I just found out its spelled fitness. And I am using the NUM_CHROMOSONES constant because the size shouldn't change anywhere, if it does the whole algorithm doesn't work so retrieving the size each time would only make it slower ?
Also : Why is it guaranteed that temp_fit will get bigger than random?
Because the random generates between 0-1 and so the random can only be as big as the total_fitnes or smaller.
[QUOTE=quincy18;19821802]Well the 'find' crap is because I threw this together quickly to see if it works, but whats wrong with having constants, this way I can easily change all the parameters when needed.[/QUOTE]
They're fine, but don't use them in your implementation, only use them in your main function and other files where you are using an interface (your code right now is completely intertwined, if you want to know why this is bad, read up on [url=http://en.wikipedia.org/wiki/Separation_of_concerns]SoC[/url]).
[QUOTE=quincy18;19821802]Random generates a number between 0-1[/QUOTE]
So make it an inline function with this prototype:
[code]inline float random(float min, float max);[/code]
That way you don't have to look at a header file to read the implementation of some code. There is no downside to using the inline function in this case since your RANDOM macro is a fully valid expression. Click [url=http://en.wikipedia.org/wiki/Inline_function#Comparison_to_macros]here[/url] to find out why inline functions are superior to macros whenever applicable.
(edit: And you really want to use double instead of float in pretty much all cases)
[QUOTE=quincy18;19821802]total_fit is the total fitnes of all the chromosome's.[/QUOTE]
Although a minor complaint; "fit" is a word on its own, your code would have better symmetry if you called it "total_fitness" (or total_fitnes).
[QUOTE=quincy18;19821802]And I am using the NUM_CHROMOSONES constant because the size shouldn't change anywhere, if it does the whole algorithm doesn't work so retrieving the size each time would only make it slower?[/QUOTE]
Firstly, speed is not a problem here. Secondly, these kind of optimizations are completely useless. You won't actually get a speed boost. For instance, the difference between passing your vector by value and passing it by reference is probably a few thousand times bigger than using a constant and a simple getter function that might even get inlined, yet, neither actually slows down your code in any significant way.
And note how you said "shouldn't"; indeed in your current program it [I]shouldn't [/I] change, but just what is it that the loop is actually supposed to do? It's supposed to loop over every member of the population vector, [I]not[/I] loop to the maximum amount of chromosomes. By using the .size getter your code is modular and won't break if you change how you use the function later on.
[QUOTE=quincy18;19821802]Also : Why is it guaranteed that temp_fit will get bigger than random?
Because the random generates between 0-1 and so the random can only be as big as the total_fitnes or smaller.[/QUOTE]
[I]You[/I] can trust that condition because you just wrote the program recently and you know what you [I]wanted[/I] it to do. An observer like myself would need to spend an unnecessarily long time figuring out the same because of your RANDOM macro, and because main is so messy it's a pain to actually predict the value of total_fit.
And while you trust your condition to at least be true once in that loop, reality indicates that that's not the case, because in some cases it reaches the end of the function.
What I'm trying to say is, spend some time working on the structure and encapsulation of the program and I can guarantee it will be a lot easier to read and debug, and a lot easier to change parts of the program without everything silently breaking down.
I am trying to improve my code and I changed allot, thanks for the info. I only have 1 problem.
They're fine, but don't use them in your implementation, only use them in your main function and other files where you are using an [B][I][U]interface[/U][/I][/B] (your code right now is completely intertwined, if you want to know why this is bad, read up on SoC).
How do I do this, do you mean I just need to make the constructor accept all constants like this :
[cpp]
class foo
{
int num,thing,another;
public:
foo(int a,int b, int c){num = a, thing = b, another = c};
}
[/cpp]
[QUOTE=quincy18;19780478]I also notice'd that the error is random, somethimes it completed 1-2 maybe 3 generations before it happened.[/QUOTE]
Tip: when you are debugging, comment out srand(), (or whatever it is seeding your random output) that way, you won't have crashes at different times and you can pin point the problem.
[QUOTE=quincy18;19960623]I am trying to improve my code and I changed allot, thanks for the info. I only have 1 problem.
They're fine, but don't use them in your implementation, only use them in your main function and other files where you are using an [B][I][U]interface[/U][/I][/B] (your code right now is completely intertwined, if you want to know why this is bad, read up on SoC).
How do I do this, do you mean I just need to make the constructor accept all constants like this :
-snip-[/QUOTE]
It's broader than that. It's a matter of modularizing and encapsulating your code. Example using the object-oriented paradigm:
[cpp]
//Foo.hpp
//Interface
class Foo
{
private:
int num, thing, another;
const int bar = 123; //constant local to class, only use if truly constant
public:
//You might want to use the body of this function to sanity check a, b and c if applicable
foo(int a, int b, int c) : num(a), thing(b), another(c) {}
void doFoo();
void doBar();
};
//Foo.cpp
//Implementation
#include "Foo.hpp"
void Foo::doFoo(){}
void Foo::doBar(){}
//main.cpp
//Client code
#include "Foo.hpp"
int main()
{
Foo foo(1, 2, 3);
foo.doFoo();
foo.doBar();
//You don't need to know what happens internally in 'foo', especially not what the constant 'bar' is, that's all localized in the implementation
return 0;
}
[/cpp]
In this way, Foo is entirely self-sufficient. Your client code cannot break it in unexpected ways: all input is clearly defined and easily traceable. It also makes Foo reusable in other projects. There are a dozen other boons you get by separating code like this; again, it's described in detail at the link.
Thats exactly how I planned it when I read you post but didn't know how to do this :
[cpp]
foo(int a, int b, int c) : num(a), thing(b), another(c) {}
[/cpp]
Thanks for all the advice you gave me.
Sorry, you need to Log In to post a reply to this thread.
