Skip to content

MAINT: Add documentation and descriptive variable names in search.py#1142

Merged
antmarakis merged 3 commits into
aimacode:masterfrom
tirthasheshpatel:patch/chap-3
Jan 2, 2020
Merged

MAINT: Add documentation and descriptive variable names in search.py#1142
antmarakis merged 3 commits into
aimacode:masterfrom
tirthasheshpatel:patch/chap-3

Conversation

@tirthasheshpatel
Copy link
Copy Markdown
Contributor

Add:

  • Docstring to __hash__ method of Node class.
  • Add descriptive names in Bidirectional search that are consistent with the paper.

Comment thread search.py Outdated
for c in problem.actions(n):
if c in open_dir or c in closed_dir:
if g_dir[c] <= problem.path_cost(g_dir[n], n, None, c):
for child in problem.actions(n):
Copy link
Copy Markdown
Contributor Author

@tirthasheshpatel tirthasheshpatel Dec 24, 2019

Choose a reason for hiding this comment

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

I am very confused here! Why is the child present in problem.actions(n) instead of n.expand(problem)?
Why does problem.actions(n) return the next states instead of actions possible in the state n?

Copy link
Copy Markdown
Contributor Author

@tirthasheshpatel tirthasheshpatel Dec 25, 2019

Choose a reason for hiding this comment

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

Inspecting further, it turns out that bidirectional search only supports GraphProblem type problems. I think we can add support for all the problem types by storing the nodes instead of states in open/closed list for both directions and calling n.expand(problem) on it to return the next states (gh-1144). Any other ideas?

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.

problem.actions(n) does return the actions possible in n, but since this is a graph problem, the actions possible are the next states. The function simply returns the neighbouring nodes, which are both the next states and actions possible. I believe though that you are right, if we want to support other types of problems, we need to switch to n.expand(problem) which is more general.

If you want, you can take this up, making bidirectional search general. That sounds like a great contribution!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yup, will fix that!

Comment thread search.py Outdated
for c in problem.actions(n):
if c in open_dir or c in closed_dir:
if g_dir[c] <= problem.path_cost(g_dir[n], n, None, c):
for child in problem.actions(n):
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.

I personally feel like c is descriptive enough, since in a lot of code c is used to denote the child.

Comment thread search.py
m, m_f = np.inf, np.inf
# pr_min_f isn't forward pr_min instead it's the f-value
# of node with priority pr_min.
pr_min, pr_min_f = np.inf, np.inf
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.

I agree, the new variable names are indeed more descriptive, nice catch!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks!

Comment thread search.py Outdated
"""Finds key in open_dir with value equal to pr_min
and minimum g value."""
m = np.inf
gmin = np.inf
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.

Here again I think that m is fine, since in what this does is a simple min search. In such code, with only one min, just writing m should be enough.

@antmarakis antmarakis merged commit 4363ddb into aimacode:master Jan 2, 2020
dj5x5 pushed a commit to dj5x5/aima-python that referenced this pull request Jul 17, 2025
…imacode#1142)

* DOC: Add docstring to __hash__ method in Node

* MAINT: Add documenation and descriptive variable names

* FIXUP: Revert to previos names
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.

2 participants