5
\$\begingroup\$

I am an architecture thesis student in my final year and I am developing a piece of software that, ideally, should design houses for me based on certain input.

I got the program to work when it was in the proof of concept phase and i was just doing three rooms, but now that I added a border and a more complicated Room graph, the algorithm seems to never reach max fitness.

using System.Collections.Generic;
using System.Linq;
using GAF;
using GAF.Operators;
using static System.Console;
using static System.Convert;
using static System.Math;

namespace RoomArrangement
{
public static class ThrowAndStick
{
    // Using Genetic Algorithms
    public static void RunThrowAndStick(this House house)
    {
        var population = new Population(100, 17 * house.Count, false, false);

        //create the genetic operators 
        var elite = new Elite(5);
        var crossover = new Crossover(0.85, true, CrossoverType.SinglePoint);
        var mutation = new BinaryMutate(0.2, true);

        //create the GA itself 
        var ga = new GeneticAlgorithm(population, chromosome => EvaluateFitness(chromosome, house));

        //add the operators to the ga process pipeline 
        ga.Operators.Add(elite);
        ga.Operators.Add(crossover);
        ga.Operators.Add(mutation);

        // Events subscription
        ga.OnGenerationComplete += ga_OnGenerationComplete;
        ga.OnRunComplete += (sender, e) => ga_OnRunComplete(sender, e, house);

        // Run the GA 
        WriteLine("Starting the GA");
        ga.Run(Terminate);
    }

    static double EvaluateFitness(Chromosome c, House house)
    {
        var fitnessList = new List<double>();

        ReadChromosome(c, house);

        // Actual Evaluation
        for(int i = 0; i < house.Count; i++)
            for(int j = i; j < house.Count; j++)
                fitnessList.Add(CompareRooms(i, j, house));

        // Check for Boundary compliance
        foreach(Room r in house)
        {
            var xFarthest = r.Anchor.X + r.Space.XDim - house.Boundary.XDim;
            if(xFarthest > 0)
                fitnessList.Add(Pow(BellCurve(xFarthest), 10));
            else
                fitnessList.Add(1);

            var yFarthest = r.Anchor.Y + r.Space.YDim - house.Boundary.YDim;
            if(yFarthest > 0)
                fitnessList.Add(Pow(BellCurve(yFarthest), 10));
            else
                fitnessList.Add(1);
        }

        fitnessList.Add(1);

        //return fitnessList.Aggregate((x, y) => x * y);
        return fitnessList.Average();
    }

    static bool Terminate(Population population,
                    int currentGeneration,
                    long currentEvaluation) => (population.MaximumFitness == 1) || currentGeneration == 5000;

    // It should compare whether two rooms intersect and if they are related, how far they are.
    static double CompareRooms(int i, int j, House house)
    {
        var returnVal = 1d;
        if(i != j)
        {
            double xRec1, yRec1, xCnt1, yCnt1;
            double xRec2, yRec2, xCnt2, yCnt2;

            var ri = house[i];
            var rj = house[j];

            ri.Read(out xRec1, out yRec1, out xCnt1, out yCnt1);
            rj.Read(out xRec2, out yRec2, out xCnt2, out yCnt2);

            double xDim = Abs(xCnt1 - xCnt2) - ((xRec1 / 2) + (xRec2 / 2));
            double yDim = Abs(yCnt1 - yCnt2) - ((yRec1 / 2) + (yRec2 / 2));

            // Related Rooms logic
            if(house.AreAdjacent(ri, rj))
                if((xDim == 0 && yDim < 0) || (xDim < 0 && yDim == 0))
                    returnVal = 1;
                else if(xDim == 0 && yDim == 0)
                    returnVal = BellCurve(1);
                else if((xDim > 0 && yDim < 0) || (xDim < 0 && yDim > 0))
                    returnVal = BellCurve(Max(xDim, yDim));
                else
                    returnVal = BellCurve(xDim) * BellCurve(yDim);

            else if(xDim < 0 && yDim < 0)
                // Intersection logic 
                returnVal = BellCurve(xDim) * BellCurve(yDim);
        }
        return returnVal;
    }

    static void ReadChromosome(Chromosome c, House house)
    {
        // Each chromosome represents a certain arrangmenet of rooms
        // a chromosome will have 17 bits total for each room:
        // 8 bits for X location , 8 bits for Y location , 1 bit for Orientation
        // A loop through the chromose would adjust the rooms accordingly.
        //
        // Example with 9 bits instead of 17, but same idea:
        // Chromosome:      000100101_001101010_110101101
        // First Room:      0001_0010_1
        // Second Room:     0011_0101_0
        // Third Room:      1101_0110_1

        // Adjusting the Rooms
        for(int i = 0; i < c.Count; i += 17)
        {
            int x = ToInt32(c.ToBinaryString(i, 8), 2);
            int y = ToInt32(c.ToBinaryString(i + 8, 8), 2);
            int oTemp = ToInt32(c.ToBinaryString(i + 16, 1), 2);

            bool o = ToBoolean(oTemp);

            house[i / 17].Adjust(x, y, o);
        }
    }

    static double BellCurve(double x)
    {
        var a = 1; // peak value
        var b = 0; // center on x-axis
        var c = 100; // width of bell curve

        return (a * Pow(E, -(Pow((x - b), 2) / (2 * Pow(c, 2))))); // Gaussian Function
    }

    // Events subscription
    static double cMax;
    static void ga_OnGenerationComplete(object sender, GaEventArgs e)
    {
        var c = e.Population.GetTop(1)[0];
        if(c.Fitness > cMax)
        {
            cMax = c.Fitness;
            WriteLine($"Fitness is {cMax}");
        }
    }

    static void ga_OnRunComplete(object sender, GaEventArgs e, House house)
    {
        var c = e.Population.GetTop(1)[0];
        ReadChromosome(c, house);

        WriteLine("The GA is Done");
        WriteLine($"Fitness is {c.Fitness}");
    }
}
}

The full code can be found here with the context and the helper methods.

\$\endgroup\$
  • \$\begingroup\$ Welcome to Code Review! Good job on your first question. \$\endgroup\$ – SirPython Apr 23 '16 at 1:18
4
\$\begingroup\$
var population = new Population(100, 17 * house.Count, false, false);  

What is 100 and 17? Magic numbers like these won't tell a reader like Bob the maintainer anything what they are about. You should extract those to some meaningful named constants.

If Bob would read house.Count Bob would just assume "hey, that can only be 1" because a house is singular.

I assume that it should represent the number of rooms avaible in the house, so you really should rename that property.


EvaluateFitness()

For each room in the house you are accessing the house objects Boundary property to access its XDim and YDim property. You should read this values into meaningful named variables outside of the loop.

//return fitnessList.Aggregate((x, y) => x * y);  

commented out code is dead code which should be removed. If you need to know how a former version of your code had been you should use some kind of source control system like git or subversion.


CompareRooms()

I could bet if you read this code in 2 months you wouldn't know what you meant by the used variables because you have used some really strange abbreviations like ri and rj`.

While we are at the naming, let us take a look into the BellCurve() method.

var a = 1; // peak value
  var b = 0; // center on x-axis
  var c = 100; // width of bell curve  

assume you would name this variables properly like

var peakValue = 1;
var xAxisCentre = 0;
var bellCurveWidth = 100;

you wouldn't need the comments anymore.

Generally speaking, naming things is hard but if you invest more time to come up with meaningful names for your things you will save that time later if you need to read (and understand) your code again.


I am never getting tired to encourage developers to always use braces {} although they might be optional. Using braces improves the readability of the code because its more structured and it leads to less error prone code.

\$\endgroup\$
  • \$\begingroup\$ Hey. Thanks for the answer. I actually have edited some parts of the code since I posted the question, which addressed some of your comments (like the BellCurve method where i just wrote the numbers in so the variables aren't assigned every time it's called.) The reason for house.Count instead of say, house.RoomCount is that this is the IList implementation, which i think I'll remove. The commented out Aggregate is there just because I keep switching between it and the average to see what gives the best results. Thanks for the comments. I'll add them when I return to the code. \$\endgroup\$ – asibahi May 14 '16 at 14:01

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.