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

Updated fov_estimator for filtering out isolated points #41

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

judyhe19
Copy link
Member

@judyhe19 judyhe19 commented Sep 3, 2024

No description provided.

@judyhe19 judyhe19 requested a review from roshambo919 September 3, 2024 17:01
@@ -97,6 +99,8 @@ def __call__(
z_min=self.z_min,
z_max=self.z_max,
)

self._eliminate_isolated_pts(pc_bev, 10, 30)
Copy link
Member

Choose a reason for hiding this comment

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

Can you make 10 and 30 input parameters to the model so we could tune them if needed? Also provide a description of what they are in the docstring.

@@ -152,6 +156,24 @@ def _estimate_fov_from_polar_lidar(
) -> "Polygon":
"""To be implemented in subclass"""
raise NotImplementedError

def _eliminate_isolated_pts(self, pc_bev, m_away, num_pts):
ptMap = defaultdict(int)
Copy link
Member

Choose a reason for hiding this comment

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

Python prefers snake case for variables, i.e. pt_map

def _eliminate_isolated_pts(self, pc_bev, m_away, num_pts):
ptMap = defaultdict(int)
usable_pts = []
for p1 in pc_bev.data.x:
Copy link
Member

Choose a reason for hiding this comment

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

You don't actually need the loops to do the distance. You can do broadcasting in numpy to make this much faster. See e.g., https://sparrow.dev/pairwise-distance-in-numpy/

Also note that the distance is commutative, meaning dist(a, b) = dist(b, a) so even if you did need loops, you could make the first loop for i in range(0, len(pc_bev.data.x)) and the second loop for j in range(i, len(pc_bev.data.x)) which would cut out half of the evaluations.

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