Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

When profits are negative, GA selects for POOR fitness #28

Open
kerbe6 opened this issue Feb 4, 2018 · 11 comments
Open

When profits are negative, GA selects for POOR fitness #28

kerbe6 opened this issue Feb 4, 2018 · 11 comments

Comments

@kerbe6
Copy link

kerbe6 commented Feb 4, 2018

Hi! I noticed that when a run has net negative fitness sum, negative profits are assigned high fitness.

The problem is this line in index.js:
selectionProb[j] = populationProfits[j] / fitnessSum;

Example: population with two members that have profits of -1 and -2:
-1 / -3 = 0.333
-2 / -3 = 0.667

I changed mine to read:

if (populationProfits[j] > 0) {
	selectionProb[j] = populationProfits[j] / fitnessSum;
}
else {
	selectionProb[j] = 1 / (populationProfits[j] * fitnessSum);
}

Additionally, the "best performer" is chosen wrong when negative profits are involved.

let maxFitness = [0, 0, 0, 0];
if (populationProfits[i] > maxFitness[0]) {
		  // Store the top performer
          maxFitness = [populationProfits[i], populationSharpes[i], populationScores[i], i];
}

My quick fix is to make this change:
let maxFitness = [-1000, 0, 0, 0];

Thanks!

@kerbe6
Copy link
Author

kerbe6 commented Feb 4, 2018

I'm actually not sure about that formula for calculating fitness on a negative profit. Might want to look into that.

@kerbe6
Copy link
Author

kerbe6 commented Feb 4, 2018

Ok, I thought long and hard, and this is the most elegant solution I could come up with:

Store a minFitness similar to how we store a maxFitness:

let minFitness = [1000, 0, 0, 0];
else if (populationProfits[i] < minFitness[0]) 

          minFitness = [populationProfits[i], populationSharpes[i], populationScores[i], i];

If we are dealing with negative profits, offset everything based on the lowest profit to ensure all values are > 0, and recalculate fitnessSum:

 if (minFitness[0] < 0) {
		let offset = 0 - minFitness[0];
		fitnessSum = 0;
		
		for (let i = 0; i < this.populationAmt; i++) {
			populationProfits[i] += offset;
			fitnessSum += populationProfits[i];
		}
	}

Calculate fitness like this, while summing the weights:

selectionProb[j] = populationProfits[j] / fitnessSum;
		sumOfWeights += selectionProb[j];`

When selecting parents to breed, we do this:
let selectedProb = randomExt.float(sumOfWeights, 0);
instead of
let selectedProb = randomExt.float(1, 0);

I also think it's a nice idea to do:

let a=0, b=0;
      while (a == b) {

to ensure we aren't breeding a parent with itself.

@illion20
Copy link

illion20 commented Mar 4, 2018

I am facing the same issue, I tried to update the code with your changes but it seems the selection is still random when profits are minus. Has there been a patch? Something in another branch that solves this?

@generalectric
Copy link
Collaborator

Nice find @kerbe6. It also needs to be fixed so that even negative profit is improved. As it is.. it will infinite loop until it has profit greater than zero, meaning we never get genes to start from. Currently it isnt a big deal with 10 loose settings and a decent strat... but if you have 20+ settings or multiple candles it starts to get a little tougher to get off the ground. There is no patch, the fix is pretty simple. Someone just needs to PR the fix or I can do it next time Im testing a strat though Im kinda backed up and have alot on the backburner.

@hugokoopmans
Copy link

@kerbe6 your adjustments are great suggestions, be aware that now the reported profits are by definition to positive!!! You add an offset to the to make them positive.

@hugokoopmans
Copy link

hugokoopmans commented Mar 13, 2018

@kerbe6 I do not understand why you do this:

When selecting parents to breed, we do this:

let selectedProb = randomExt.float(sumOfWeights, 0);

instead of

let selectedProb = randomExt.float(1, 0);

The selectProb is already a proportion, a probability should be between 0 and 1 so why the extra step?

@hugokoopmans
Copy link

my suggestion is to split populationProfit and populationFitness so we can create a custom fitness function regardless of the actual profits a strat makes...

@mboremski
Copy link

could there be (as a first step) a way to keep the best profit even if negative?
If marked is going down and down in a timespan this might be good way to minimize losses...

@mboremski
Copy link

Any news here?

@generalectric
Copy link
Collaborator

I've visited this a couple times without solving it. I just haven't had the time to figure it out and got my hands full at the moment.

@mboremski
Copy link

could there be (as a first step) a way to keep the best profit even if negative?
If marked is going down and down in a timespan this might be good way to minimize losses...

Any news?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants