Skip to content

Add HillClimbingSearchWithSidewaysMoves.java - #358

Closed
ritwik12 wants to merge 1 commit into
aimacode:AIMA3efrom
ritwik12:AIMA3e
Closed

ritwik12 wants to merge 1 commit into
aimacode:AIMA3efrom
ritwik12:AIMA3e

Conversation

@ritwik12

@ritwik12 ritwik12 commented Mar 5, 2018

Copy link
Copy Markdown
Contributor

I am adding HillClimbingSearchWithSidewaysMoves.java from AIMA4e for the issue #357
The code was originally written by Ciaran O'Reilly.
I think this code is important as SidewaysMoves are very helpful in a case of a Plateaux/shoulder.

I am adding HillClimbingSearchWithSidewaysMoves.java from AIMA4e.
The code was originally written by Ciaran O'Reilly.
I think this code is important as SidewaysMoves are very helpful in a case of a Plateaux/shoulder.
@utsavoza

utsavoza commented Mar 6, 2018

Copy link
Copy Markdown
Contributor

I am unsure of how all the checks have passed successfully for this PR. I suppose ant aima-core.test-all in .travis.yml currently validates only changes made to aima-core. As for the changes are concerned, the implementation doesn't comply with the search api in AIMA3e branch. I'd rather accompany the implementation with a unit test to be sure of the changes made to a specific branch.

@ritwik12

ritwik12 commented Mar 6, 2018

Copy link
Copy Markdown
Contributor Author

I have reimplemented the code from AIMA4e branch and up to my knowledge that code works well.
Can you please elaborate what you are suggesting?

@ritwik12

Copy link
Copy Markdown
Contributor Author

@norvig Please have a look

@samagra14 samagra14 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The absence of appropriate tests is disturbing. Please add them.

@norvig

norvig commented Jul 22, 2018

Copy link
Copy Markdown
Member

The implementation is faulty and there are no test cases. Besides, there is no requirement of Hill Climbing with sideways moves as far as AIMA3e is concerned.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants