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

Add example demonstrating spike-based Sudoku solver #2384

Merged
merged 42 commits into from
Aug 2, 2022

Conversation

krachbumm3nte
Copy link
Contributor

A NEST implementation of Sudoku as a constraint satisfaction problem based on (https://github.com/SpiNNakerManchester/IntroLab/tree/master/sudoku)

@heplesser heplesser changed the title add sudoku example Add example demonstrating spike-based Sudoku solver May 4, 2022
@heplesser heplesser added T: Enhancement New functionality, model or documentation S: Normal Handle this with default priority I: No breaking change Previously written code will work as before, no one should note anything changing (aside the fix) labels May 4, 2022
@heplesser heplesser added this to In progress in Documentation via automation May 4, 2022
Copy link
Contributor

@heplesser heplesser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@krachbumm3nte Thank you for porting this exciting example to NEST. I noticed that some of your scripts reference S Furber and A Rowley as authors. I am therefore wondering to what degree your code is written from scratch and to which degree it is modified from existing code by Furber/Rowley? I believe their original implementation was in PyNN.

We need to make sure we do the right thing with respect to copyright and licensing.

I enter this comment as "request changes" to ensure that we won't merge before we have clarified this point.

Documentation automation moved this from In progress to Review May 4, 2022
@krachbumm3nte
Copy link
Contributor Author

@heplesser The actual code shares very little resemblance to the original implementation. The parts I copied are the connection statistics, neuron parameters and the exemplary sudoku puzzles (helpers.py/get_puzzle()).
I also contacted both of the original authors and got permission to publish my version with their names attached where relevant.

Copy link
Contributor

@jessica-mitchell jessica-mitchell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this contribution @krachbumm3nte, really cool example! I noted a few things to fix up with the formatting and such. Please make sure to fix conflicts first before applying reviewer changes.

pynest/examples/sudoku/README.rst Outdated Show resolved Hide resolved
pynest/examples/sudoku/README.rst Outdated Show resolved Hide resolved
pynest/examples/sudoku/helpers.py Outdated Show resolved Hide resolved
pynest/examples/sudoku/helpers.py Outdated Show resolved Hide resolved
pynest/examples/sudoku/helpers.py Outdated Show resolved Hide resolved
pynest/examples/sudoku/plot_progress.py Outdated Show resolved Hide resolved
pynest/examples/sudoku/plot_progress.py Outdated Show resolved Hide resolved
pynest/examples/sudoku/helpers.py Outdated Show resolved Hide resolved
pynest/examples/sudoku/sudoku_net.py Outdated Show resolved Hide resolved
pynest/examples/sudoku/README.rst Outdated Show resolved Hide resolved
krachbumm3nte and others added 16 commits May 16, 2022 11:12
Co-authored-by: jessica-mitchell <mitchell20j@gmail.com>
Co-authored-by: jessica-mitchell <mitchell20j@gmail.com>
Co-authored-by: jessica-mitchell <mitchell20j@gmail.com>
Co-authored-by: jessica-mitchell <mitchell20j@gmail.com>
Co-authored-by: jessica-mitchell <mitchell20j@gmail.com>
Co-authored-by: jessica-mitchell <mitchell20j@gmail.com>
Co-authored-by: jessica-mitchell <mitchell20j@gmail.com>
Co-authored-by: jessica-mitchell <mitchell20j@gmail.com>
Co-authored-by: jessica-mitchell <mitchell20j@gmail.com>
Co-authored-by: jessica-mitchell <mitchell20j@gmail.com>
Co-authored-by: jessica-mitchell <mitchell20j@gmail.com>
Co-authored-by: jessica-mitchell <mitchell20j@gmail.com>
Co-authored-by: jessica-mitchell <mitchell20j@gmail.com>
krachbumm3nte and others added 11 commits May 16, 2022 11:29
pynest/examples/sudoku/sudok_net.py
Co-authored-by: jessica-mitchell <mitchell20j@gmail.com>
Co-authored-by: jessica-mitchell <mitchell20j@gmail.com>
Co-authored-by: jessica-mitchell <mitchell20j@gmail.com>
Co-authored-by: jessica-mitchell <mitchell20j@gmail.com>
Co-authored-by: jessica-mitchell <mitchell20j@gmail.com>
Co-authored-by: jessica-mitchell <mitchell20j@gmail.com>
Co-authored-by: jessica-mitchell <mitchell20j@gmail.com>
Co-authored-by: jessica-mitchell <mitchell20j@gmail.com>
Copy link
Contributor

@jessica-mitchell jessica-mitchell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @krachbumm3nte. I have one small formatting adjustment, but I will approve. I would like to make the gif more visible, but I think it makes sense to do that with / after #2289.

pynest/examples/sudoku/plot_progress.py Show resolved Hide resolved
Co-authored-by: jessica-mitchell <mitchell20j@gmail.com>
krachbumm3nte added a commit to krachbumm3nte/nest-simulator that referenced this pull request Jun 24, 2022
@gtrensch gtrensch self-requested a review August 1, 2022 12:38
Copy link
Contributor

@gtrensch gtrensch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for porting this example to NEST. 👍
I approve the PR, but also would have a minor comment: While testing the example, I went into a dependency issue related to the Python imaging library PLI. Discussed this with @terhorstd, and we would like to replace PLI with a matplotlib solution at a later time. We opened #2441 to follow up on this.

@terhorstd terhorstd removed the request for review from heplesser August 2, 2022 06:43
@terhorstd terhorstd dismissed heplesser’s stale review August 2, 2022 06:44

The point in question was answered, re-request of review was unanswered, replaced reviewer.

@terhorstd terhorstd merged commit 052f9f0 into nest:master Aug 2, 2022
Documentation automation moved this from Review to Done Aug 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I: No breaking change Previously written code will work as before, no one should note anything changing (aside the fix) S: Normal Handle this with default priority T: Enhancement New functionality, model or documentation
Projects
Documentation
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

5 participants