| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
|
@MrDupin Would it make sense to replace I only suggest this because then the code will truly mirror the pseudocode, which is desirable in my opinion. It won't have the extra lines and different parameter lists like it does right now. I would be happy to make this change if you like, else you can just add it to this PR. |
Sorry, something went wrong.
|
@kaivalyar: This is more or less how I had it initially, but as per Darius' suggestion, I changed the implementation to the current version due to performance issues. It would take a very long time to complete due to the added computational cost. The difference in performance was quite astute and older machines could previously run only small and trivial-ish problems. There are, as I see it, three main ways to go about it: a) Keep it as is, choosing performance over two additional lines of code + the function parameters of random_selection. b) Follow the pseudocode to the letter, writing some possibly ugly and harder to follow code on the side to relieve the computational pressure. c) Keep it as is, but change the fitnesses = map(fitness_fn, population) random_selection = weighted_sampler(population, fitnesses) part to something like: random_selection = weighted_sampler(population, map(fitness_fn, population)) Which will reduce the additional lines to just 1. It is a bit uglier in my opinion though. Whichever way we choose, we should definitely have random_selection calculated only once. The first (current) and third options already accomplish that. For the second to work, we have to define random_selection as a function that on its first call will compute weighted_sampler and return an item, but on any subsequent calls it won't recomputed the sampler, instead just returning an item. Note that there should be a way to "reset" this, so at every generation it actually takes into account the new fitnesses. That possibly means adding another line/altering the pseudocode, or making the function even more convoluted. Personally, I believe we should leave it as is. The only difference from the pseudocode is the two additional lines to compute random_selection and the fact that the calls to random_selection inside don't pass parameters. I believe it is as readable as the pseudocode and the differences aren't that prominent. |
Sorry, something went wrong.
|
Woah. This turned out to be a lot more complicated than I imagined. I think you're right, let's stick with the way things are currently. We can always further update the code later. In the meanwhile, let me explore if this can be solved differently somehow... |
Sorry, something went wrong.
|
No problem. If you need any help/feedback/discussion, reach out to me. I would love to help any way I can. |
Sorry, something went wrong.
|
There is another, more elegant solution, now that I think about it. Define a function that will calculate the fitnesses and return the weighted_sampler. def selection_chance(fitness_fn, population)
fitnesses = map(fitness_fn, population)
return weighted_sampler(population, fitnesses)
And then inside the generations loop, replace fitnesses = map(fitness_fn, population) random_selection = weighted_sampler(population, fitnesses) with random_selection = selection_chance(fitness_fn, population). I believe this is overall a better solution. It makes the code closer to the pseudocode while also making it more readable. I will make this change (assuming no other solution is found till then) once this PR gets merged. |
Sorry, something went wrong.
|
I think @MrDupin has a good solution for now. If/when we go to Python 3.6, there is a new random.choices function which does a similar thing. |
Sorry, something went wrong.
* Update search.py * Update test_search.py
| Back | FazBrowse Home | New Git URL |
As per the discussion on #347, I changed the individuals from string to list. Updated tests accordingly.
I broke the Graph Coloring test into three mini-tests, one for each of the three main variable types (char/string, integer, boolean).
NOTE: I have updated the notebook too, but I am waiting till #522 gets merged to avoid dealing with conflicts.