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

Implemented A* algorithm and updated test cases #576

Closed
wants to merge 8 commits into from

Conversation

nagajaideep
Copy link
Contributor

@nagajaideep nagajaideep commented Jan 16, 2025

References to other Issues or PRs or Relevant literature

fixes #486
https://github.com/codezonediitj/pydatastructs/pull/283/files
https://github.com/codezonediitj/pydatastructs/pull/301/files

Brief description of what is fixed or changed

1.implemented a star algorithm with the help of Manhattan heustric and took reference from the Wikipedia
2.took reference from the Dijkstra and bellman ford algorithm for test cases and code writing

Other comments

@nagajaideep nagajaideep deleted the Astar branch January 16, 2025 15:44
@nagajaideep nagajaideep restored the Astar branch January 16, 2025 16:30
@nagajaideep nagajaideep reopened this Jan 16, 2025
@nagajaideep
Copy link
Contributor Author

@Kishan-Ved @czgdp1807
image
this issue in the terminal isn't a changed by me but it's about an import error .any idea on how to resolve this?

Comment on lines 756 to 762
if algorithm == 'a_star_with_manhattan':
# A* with this implementation requires both source and target
if not target:
raise ValueError("Target must be specified for A* algorithm")

func = "_a_star_with_manhattan_" + graph._impl
return getattr(algorithms, func)(graph, source, target)
Copy link
Member

Choose a reason for hiding this comment

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

This is not needed. The logic below already handles this.

@@ -293,7 +292,34 @@ def _test_shortest_paths_positive_edges(ds, algorithm):
graph.remove_edge('SLC', 'D')
graph.add_edge('D', 'SLC', -10)
assert raises(ValueError, lambda: shortest_paths(graph, 'bellman_ford', 'SLC'))

def _test_a_star_manhattan(ds):
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
def _test_a_star_manhattan(ds):
def _test_a_star_manhattan(ds):

Copy link
Contributor Author

Choose a reason for hiding this comment

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

should i remove this tests for the astar algorithem here?

Copy link
Contributor

Choose a reason for hiding this comment

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

This suggestion means that you need to add a blank line before the function definition starts.

@czgdp1807
Copy link
Member

this issue in the terminal isn't a changed by me but it's about an import error .any idea on how to resolve this?

It's a path setting issue in VSCode. You need to add PATH to your conda environment headers in VSCode settings.

@nagajaideep nagajaideep requested a review from czgdp1807 February 8, 2025 13:13
@nagajaideep
Copy link
Contributor Author

nagajaideep commented Feb 8, 2025

@czgdp1807 I have implemented the requested changes. However, the failing GitHub CI/CD tests appear to be due to a different import error related to the C++ node modules. Could you please verify if this issue is caused by my changes?

@Kishan-Ved
Copy link
Contributor

@czgdp1807 I have implemented the requested changes. However, the failing GitHub CI/CD tests appear to be due to a different import error related to the C++ node modules. Could you please verify if this issue is caused by my changes?

Are the tests working on your local machine? Check the README to know how to run the tests.

@Kishan-Ved
Copy link
Contributor

Kishan-Ved commented Feb 8, 2025

Inorder to debug, do this:

  1. Add raise_if_backend_is_not_python() in your function (check other functions above/below to know how this is done)
  2. Remove the tests, commit -> See if the CI checks pass
  3. a) If CI checks pass -> add the tests.
  4. b) If CI checks fail -> This means that there is some issue with the files modified by your commit

@nagajaideep
Copy link
Contributor Author

nagajaideep commented Feb 10, 2025

@Kishan-Ved @czgdp1807
I’ve removed the tests as suggested, but the CI/CD checks are still failing. The error seems to be related to the C++ node modules. I’ve also checked the README and tried running the tests locally(same errors are popping up). Could you help verify if this issue is caused by my changes(before these errors were not coming up) or something else in the repository?

1.one thing is that i can re-fork my local repo and give a new PR ?

please let me if any other method are there to resolve this ASAP..

@@ -24,6 +25,7 @@
'topological_sort',
'topological_sort_parallel',
'max_flow'
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
'max_flow'
'max_flow',

Copy link
Contributor

Choose a reason for hiding this comment

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

Please add a comma here.

@@ -11,6 +11,7 @@
from pydatastructs.graphs.graph import Graph
from pydatastructs.linear_data_structures.algorithms import merge_sort_parallel
from pydatastructs import PriorityQueue
from pydatastructs.graphs.graph import AdjacencyListGraphNode
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
from pydatastructs.graphs.graph import AdjacencyListGraphNode

Copy link
Contributor

@Kishan-Ved Kishan-Ved Feb 10, 2025

Choose a reason for hiding this comment

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

I see that you have added this import, but it is being used in a code block that is commented. Please try removing this and check if the CI tests pass.

@@ -11,6 +11,7 @@
from pydatastructs.graphs.graph import Graph
from pydatastructs.linear_data_structures.algorithms import merge_sort_parallel
from pydatastructs import PriorityQueue
from pydatastructs.graphs.graph import AdjacencyListGraphNode
Copy link
Contributor

@Kishan-Ved Kishan-Ved Feb 10, 2025

Choose a reason for hiding this comment

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

I see that you have added this import, but it is being used in a code block that is commented. Please try removing this and check if the CI tests pass.

@nagajaideep
Copy link
Contributor Author

nagajaideep commented Feb 14, 2025

@Kishan-Ved
The issue still persists, so I will create a completely new pull request(with all the changes requested) and refork the repository, as that seems to be the only remaining option. Please let me know if there is an alternative solution.

@@ -811,6 +818,55 @@ def _dijkstra_adjacency_list(graph: Graph, start: str, target: str):

_dijkstra_adjacency_matrix = _dijkstra_adjacency_list

def _a_star_with_manhattan_adjacency_list(graph: Graph, start: str, target: str, **kwargs):
Copy link
Contributor

Choose a reason for hiding this comment

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

Try changing the body of this function to pass. Check if the CI checks pass. Else, make a new PR with small code additions and one commit at a time, and ensure CI passes.

@nagajaideep nagajaideep closed this by deleting the head repository Feb 14, 2025
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.

A* Algorithm
3 participants