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

Utilize numpy to reduce calculation time #767

Merged
merged 5 commits into from Jan 8, 2023

Conversation

kadirhas
Copy link
Contributor

@kadirhas kadirhas commented Dec 30, 2022

Change-Id: I6e421a1c2524a3d8f8875121a1a6d2ed832c3150

Reference issue

What does this implement/fix?

I've noticed that the current Implementation uses lists. By replacing them with numpy arrays and using built in numpy functions such as any, I reduced the calculation time of d_star_lite. The other thing that I've noticed, once after initializing class, if the user wants to add new spoofed objects and calculate a new path, it was not using the prior information that is generated. So I modified the initialization process a bit to use the prior information.

Additional information

CheckList

  • Did you add an unittest for your new example or defect fix?
  • Did you add documents for your new example?
  • All CIs are green? (You can check it after submitting)

Kadir Haspalamutgil added 2 commits December 30, 2022 09:31
Change-Id: I6e421a1c2524a3d8f8875121a1a6d2ed832c3150
Change-Id: I909d49b40e6792efcb796846c97a0d984471d3c9
@kadirhas
Copy link
Contributor Author

The styling warnings are caused by W503, which says that it is going to change.

Copy link
Owner

@AtsushiSakai AtsushiSakai left a comment

Choose a reason for hiding this comment

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

Thank you for your PR. I have some points to improve the readability of this code. PTAL.

) or (
self.rhs[self.start.x, self.start.y]
!= self.g[self.start.x, self.start.y]
).any():
Copy link
Owner

Choose a reason for hiding this comment

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

Not this PR topic, but I think each while condition is challenging to read, so is it possible to introduce boolean variables or other kinds of intermediate values for readability?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is a great suggestion, thank you! I've updated this as you have suggested. Please take a look.

if self.detected_obstacles_xy.shape[0] > 0:
is_in_detected_obstacles = (
(self.detected_obstacles_xy[:, 0] == x)
& (self.detected_obstacles_xy[:, 1] == y)
Copy link
Owner

Choose a reason for hiding this comment

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

Difficult to read each condition, is it possible to improve it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I separated the conditions with boolean variables.

self.g = np.full(
(self.x_max, self.y_max),
float("inf") # type: ignore
)
self.detected_obstacles = list() # type: ignore
Copy link
Owner

Choose a reason for hiding this comment

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

self.dtected_obstacles is still needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is still used in random object generation. I will change that function as well and test it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed it, and while doing that fixed a bug about going out of range of the rhs.

self.rhs = np.full(
(self.x_max, self.y_max),
float("inf") # type: ignore
)
Copy link
Owner

Choose a reason for hiding this comment

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

I think introducing a new function to initialize a grid map is better like:

def init2dGrid():
       return np.full(self.x_max, self.y_max), float("inf")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I used an already existing function called create_grid. Thanks for the improvement.

@@ -52,6 +53,8 @@ class DStarLite:
Node(-1, -1, math.sqrt(2))
]

initialized = False
Copy link
Owner

Choose a reason for hiding this comment

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

This should be initizlied in the __init__?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I moved it, thank you

Kadir Haspalamutgil added 3 commits January 3, 2023 09:02
Change-Id: Ic329c2140f2200b49feeb03c588d7a805b96cbdc
Change-Id: I4a0f2a424f17f44de3b444cb9c2e8e715d761d34
Change-Id: Id55c5972cbe76a2f6a69527dbf770f9bdf059109
Copy link
Owner

@AtsushiSakai AtsushiSakai left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you for great PR!!

@AtsushiSakai AtsushiSakai merged commit 98d5851 into AtsushiSakai:master Jan 8, 2023
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