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 support for complex actions #304

Closed

Conversation

Manthan-R-Sheth
Copy link
Contributor

@Manthan-R-Sheth Manthan-R-Sheth commented Apr 2, 2017

The method of reversing and adding to the list is changed to support complex actions in the problem.

  1. fromMeetingStateToInitialStateList and fromMeetingStateToGoalStateList are made to achieve the purpose.
  2. InitialState and GoalState are included in the fromGoalStateToInitialState and fromInitialStateToGoalState respectively.

@Manthan-R-Sheth
Copy link
Contributor Author

@ctjoreilly Fixes #298

@ctjoreilly
Copy link
Contributor

Hi,

Could you update this with respect to the recent changes from pull request #308. Also, please ensure you squash/rebase as needed.

Thanks

Ciaran

@Manthan-R-Sheth
Copy link
Contributor Author

@ctjoreilly
The implementation in #308 increases complexity of the whole algorithm (as an additional O(n) algorithm is used to find the meeting node). Though other changes i.e. name changes and addition of test cases are fine, the changes in BidirectionalSearch aren't (Correct me if I am wrong).
Moreover after this PR (#304) is included in the main code, same results are obtained as that in #308 and with the null checks, all would be good to go.

@ctjoreilly
Copy link
Contributor

Hi @Manthan-R-Sheth and @globalworming,

I have moved and renamed BidrectionalSearch from the core to extra module under:

aima.extra.search.uninformed.BidirectionalSearchGW

@Manthan-R-Sheth I am going to close this pull request but could you do the following:

  • Add your version of BidrectionalSearch to aima.extra.search.uninformed.BidirectionalSearchMRS
  • Update BidirectionalSearchTest to include "BidirectionalSearchMRS" as searchFunctionName and the searchForActions() method is updated to instantiate it correctly. You version needs to pass all the current tests.
  • Update the README.md to include a link to your version of the algorithm (I have added a TODO in there for you).
  • Do not modify any of the core code with this pull request - i.e. BasicBidirectionalSearchResult, SearchForActionsBidirectionallyFunction, etc... should remain untouched.

My rationale for this re-org is as follows:

  1. Neither implementation belongs within the core module as they do not reflect what is currently outlined in: 4th Edition Algorithm: Bidirectional Search aima-pseudocode#41. In particular as it is an informed (i.e. heuristic) based algorithm that is described.

  2. Due to 1 my focus has not been on the current implementations that have been submitted but on the API.

  3. I appreciate both your efforts and think it good/healthy to place both alternative methods in the 'extra' module so their relative trade offs can be compared more easily side by side.

Thanks

Ciaran

@ctjoreilly ctjoreilly closed this Apr 3, 2017
@Manthan-R-Sheth
Copy link
Contributor Author

@ctjoreilly
This sounds logical 👍 and I shall do the required in the upcoming PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants