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

Added Sudoku example #2021

Closed
wants to merge 9 commits into from

Conversation

Projects
None yet
4 participants
@satyaki3794
Copy link
Contributor

commented Mar 11, 2016

I have added an example for sudoku solver to hpx/examples. The initial state of the board is fixed and the solver uses brute force/backtracking to solve the puzzle or reports if no solution is possible.


add_hpx_component(sudoku
SOURCE_GLOB "sudoku.cp*"
HEADER_GLOB "sudoku.hp*"

This comment has been minimized.

Copy link
@zao

zao Mar 11, 2016

Contributor

Any particular reasons for the use of globs here, or is it inspired by an example?

This comment has been minimized.

Copy link
@satyaki3794

satyaki3794 Mar 11, 2016

Author Contributor

Inspired by nqueens example.

@hkaiser hkaiser added this to the 0.9.12 milestone Mar 11, 2016

: public hpx::components::component_base<board>
{
private:
list_type list_;

This comment has been minimized.

Copy link
@hkaiser

hkaiser Mar 11, 2016

Member

Why calling it a 'list' if it's anything but one?

This comment has been minimized.

Copy link
@satyaki3794

satyaki3794 Mar 11, 2016

Author Contributor

The 9X9 board is represented as a list of 81 elements. Position (i, j) on the board corresponds to (9*i+j) in the list (0-indexed).

This comment has been minimized.

Copy link
@hkaiser

hkaiser Mar 11, 2016

Member

But you're storing it as a std::vector, not a std::list, aren't you? Also, a std::vector<boost::uint8_t> should be sufficient for your use case (you're storing numbers in the range [0,81] only).

This comment has been minimized.

Copy link
@satyaki3794

satyaki3794 Mar 11, 2016

Author Contributor

Okay. I will change the name of the attribute and data_type to uint8_t. Also, it stores numbers in the range [0,9].

#include <hpx/runtime/threads/thread_data.hpp>
#include <hpx/runtime/components/component_type.hpp>
#include <hpx/runtime/actions/component_action.hpp>
#include <hpx/runtime/serialization/serialize.hpp>

This comment has been minimized.

Copy link
@hkaiser

hkaiser Mar 11, 2016

Member

Please try to include headers from hpx/include instead, if possible. We'd like to slowly move away from exposing the internal headers.

#if !defined(HPX_SUDOKU_EXAMPLE_SERVER)
#define HPX_SUDOKU_EXAMPLE_SERVER

#include <iostream>

This comment has been minimized.

Copy link
@hkaiser

hkaiser Mar 11, 2016

Member

Is including this header really necessary?

This comment has been minimized.

Copy link
@satyaki3794

satyaki3794 Mar 11, 2016

Author Contributor

I've used std::cout and std::endl in this file.

// file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt)


#include <iostream>

This comment has been minimized.

Copy link
@hkaiser

hkaiser Mar 11, 2016

Member

Would it be viable to use hpx::cout instead of the standard streams?

@hkaiser

This comment has been minimized.

Copy link
Member

commented Mar 11, 2016

Overall, very nice. I'd prefer to move the initialization of the board into the main program, though. The way you implemented it the component can be used to solve only a single puzzle. It would be nice it solved arbitrary ones.

@satyaki3794

This comment has been minimized.

Copy link
Contributor Author

commented Mar 11, 2016

The initialization function throws a 'future has no valid shared state' exception, similar to nqueens one (please refer here: #802 (comment)). I tried fixing it, but was unable to. I found only a mention of this in http://stellar-group.github.io/hpx/docs/html/hpx/error.html, but not any more details.

satyaki3794 added some commits Mar 11, 2016

@hkaiser

This comment has been minimized.

Copy link
Member

commented Mar 15, 2016

Several general comments:

  • Where is the parallelism? Wasn't the plan to solve the puzzle using some parallelization?

  • Please get rid of the stubs, we deprecated those (just move the async calls directly into the client)

  • The current code throws an exception saying that this future has no valid shared state: HPX(no_state), this is because the code does not actually initialize the board (it doesn't create an instance of that component). Either do

    sudoku::board new_board = hpx::new_<sudoku::board>(locality_);
    

    or do somethiing equivalent in the constructor of the client object.

  • As said before, I'd prefer for the component not to initialize the board with some concrete puzzle, I think this should be done from the application via the constructor of the puzzle object.

  • The 'solver' should be part of the puzzle component as well and should be exposed through a solve action or somesuch. In the end, the application would look like

    sudoku::board_data data = { ... };
    sudoku::board board = hpx::new_<sudoku::board>(hpx::find_here(), board_data);
    hpx::future<sudoku::board_data> solution = board.solve();
    
    for (sudoku::cell const& c : solution.get())
        ...
    

    (or similar).

@satyaki3794

This comment has been minimized.

Copy link
Contributor Author

commented Apr 7, 2016

I have implemented the suggested changes. Removed the deprecated stubs and handled the future_has_no_shared_state_exception. Added the feature of taking the starting board configuration from the user along with a default which is provided. Changed the implementation logic of solve_board(). It now returns a future containing the final board configuration, and uses hpx::async() to create new threads asynchronously, and cancellation tokens to signal them to stop, if the parent thread has already found a solution.

@hkaiser

This comment has been minimized.

Copy link
Member

commented May 3, 2016

@satyaki3794 Thanks for all of your work on this and sorry for the long delay in responding.

Everything looks fine now except one thing which you might want to consider changing. Currently the code which asks the user for an initial board layout sits in the component. I think this belongs into the client executable instead. The component is really just the solver, it gets initialized from an initial board layout (wherever that comes from) and returns a solution.

@satyaki3794

This comment has been minimized.

Copy link
Contributor Author

commented May 19, 2016

Do you think the whole functionality should be in the client executable? Or can I take the input via the client and pass a vector to the action which initializes the board (as opposed to doing everything inside the component action), in which case the component action simply assigns the board config to the one passed as an argument?

@sithhell

This comment has been minimized.

Copy link
Member

commented Jun 22, 2016

What's left to do here?

@hkaiser

This comment has been minimized.

Copy link
Member

commented Jun 22, 2016

What's left to do here?

See my comment above:

Everything looks fine now except one thing which you might want to consider changing. Currently the code which asks the user for an initial board layout sits in the component. I think this belongs into the client executable instead. The component is really just the solver, it gets initialized from an initial board layout (wherever that comes from) and returns a solution.

@hkaiser

This comment has been minimized.

Copy link
Member

commented Jun 26, 2016

I'm going to close this without merging. Too many things would have to be changed for this to be a usable example.

@hkaiser hkaiser closed this Jun 26, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.