-
-
Notifications
You must be signed in to change notification settings - Fork 334
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
New Sudoku extension and GTK# sample #43
Conversation
…GTK sample with file support and multiple strategies
As per your comment in the other current pull request, I added some code coverage demonstrating using the various strategies. In order to keep the test fast, I only tested for complete resolution with the fastest strategy. |
OK I hope the coverage has got satisfying without taking too much time. Now I have remarks and questions: Sudoku has got many local maxima, which makes it hard for GAs, and I believe a good test for your library. Looking at existing documented implementations, it seems most have gone with a simple 1 cell = 1 gene implementation, and they would usually acknowledge the local maximum issue. In order to get comparison material, I included in the default GTK sample the 2 sudokus documented on this page (easy) and that one (medium), in addition to one easier and one harder than those two. Using a relatively efficient Permutation based chromosome, I am pleased that all Sudokus seem to be tractable with the current implementation. The "easy" one requires approximately 10 secs, which is satisfying as compared to the 15 minutes mentioned in the article. However, I figured the only efficient way I found to escape local maxima was to increase the GA's population depending on the Sudoku's difficulty, and that both with the permutation and the more naive cell-based strategies. Basically, if the Sudoku wasn't solved by the 50th generation, there would be nearly no chances the GA escapes its sub-optimal solution in any other number of generations.
One could argue that the default settings with Elite Selection and reinsertion is responsible for that, but I tried hard to play with all parameters and could not find any better setup. In particular, setting the crossover probability to one to discard parents would yield getting stuck on lower quality local maxima. Then I introduced 2 alternatives:
Those strategies were somehow partially successful at trading population size for generation numbers, playing with the GA settings but I feel this is not quite satisfying, since in all other implementations I have seen, the population size is vastly smaller than ours. So here goes my questions:
|
Thanks for this pull-request, it will be amazing to have a Sudoku sample using GeneticSharp. Your Multiple Chromosome and Random permutations are strong candidates to be moved from Extensions to GeneticSharp core in the future. About your questions:
I will need to take some time to study your implementation and try some configurations. As your pull-request is quite large, I will need several days of my spare time to review and test it, but no worry I'will review it as soon as possible. |
…erate Sudoku unit tests.
Thanks for you prompt answer.
It is a regular course in AI I teach in several French engineering school. An earlier version used to be online, but it is now mainly contained in the various LMS for those schools. Hopefully I can wrap it up again online some time next year. By the way, I have other samples I would gladly propose as pull requests, but I thought this one was the easier pick to get started. Another one is a bit similar to that project and I reckon it does a better job at demonstrating GAs for image processing than the existing bitmap equality sample. Anyway, I suppose we can discuss that once the Sudoku's sample is properly integrated.
Well I'll gladly help with that. They are still pretty experimental for now, but they serve a purpose that can be extended to other cases indeed.
Sure, thanks for having a look. Looking forward to learning how best to use your library. |
Hmmm, not sure what to think about the last continous-integration test fail. Well it's not my code for sure, but as I realized myself, one should be careful with putting hard limits on unit tests, especially on time measurements, considering you don't really know what's going to happen on the continous-integration VM. Also, for precise time measurements, one should use a stopwatch, so if you don't mind, I'll commit a change to account for that. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
List<T> properties
Please, change all public properties changing LList<T> to use IList<T>.
Here some discussions about:
Sudoku class name
https://docs.microsoft.com/pt-br/dotnet/standard/design-guidelines/names-of-namespaces
DO NOT use the same name for a namespace and a type in that namespace.
Maybe the class could be called SudokuBoard
.
Sudoku Parse and ParseFile
It will be good to have the expected format of string and file described in some place.
Some suggestions:
- In a help button on GTK sample
- In the method documentation
- In a section on the GeneticSharp wiki
Or any other idea you can think about.
Solve issues
Please, solve the issues listed on https://sonarcloud.io/project/issues?branch=MyIntelligenceAgency-develop&id=GeneticSharp&resolved=false
Next steps
Please, feel free to question about any of the sections I pointed above.
After you finish the changes, I will perform another review.
And finally, I think we will merge this pull-request and after I will start a feature branch to try to answer your questions below:
- Can you think of any set of parameters that make the current implementation more efficient?
- How would you explain other GAs libraries seemingly able to work with lower populations and yield progresses over higher generation numbers?
After we get some answer, we can take a look into your other samples proposes.
Let's evolve!
OK I think I covered all your requests. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unit tests naming
This section I forgot to mention before, but I need you to follow the naming convention described in this link https://github.com/giacomelli/GeneticSharp/wiki/unit-tests#naming to name your unit tests.
Sudoku Parse and ParseFile
I did not find any change about this one. Let me know if I miss something.
It will be good to have the expected format of string and file described in some place.
Some suggestions:
- In a help button on GTK sample
- In the method documentation
- In a section on the GeneticSharp wiki
Or any other idea you can think about.
Solve issues
There are still 5 remaining issues.
Please, solve the issues listed on https://sonarcloud.io/project/issues?branch=MyIntelligenceAgency-develop&id=GeneticSharp&resolved=false
This is the option that was implemented:
Ok now your turn! Note that considering the level of scrutiny of your review and your tools', my expectations are pretty high... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Almost there!
There is only one issue to be solved: https://sonarcloud.io/project/issues?branch=MyIntelligenceAgency-develop&id=GeneticSharp&open=AWbMRnGTSgN4uCcl1Kdi&resolved=false
You need to cover with unit tests all lines that are marked with red color.
Here you go ! |
You still need to improve the code coverage of SudokuBoard (most of them in ToString method) and there is another issue at MultipleFitness. |
OK I updated MultipleFitness, but the other issue seem like a bug in Sonarcloud: it points at the exact same lines as before I did actually provide coverage for all of them (Exception in ctor, SetCell, ToString & ParseFile): look for yourself: the file is 5 lines longer and the lines are out of sync now in Sonarcloud now: it even asks to cover comments for the ParseFile method, which does not make any sense. |
Ok, thanks. I will merge it do develop now. I will create ASAP a release branch to release it to master. |
OK Cool. Now her are additional thoughts to help with your investigation:
|
Please, start a new question issue referencing this pull-quest, adding your original questions and those additional thoughts above. I'll give some attention to this issue in the next weekends. Thanks! |
|
I propose the addition of a Sudoku solver as a new set of classes in the Extensions project, and the corresponding sample in the GTK App.
This was made as part of a course I teach where GeneticSharp is used for GAs.
Sudoku is arguably a good example, because it is relatively hard to solve with GAs whereas it is much more tractable with other tools. See for instance those LINQ examples with Microsoft's Z3 solver or a dedicated constraint based solver.
The extensions material include:
The GTK sample includes: