Khalil Hawari Bootcamp #139

Khalil-Hawari · 2025-01-08T23:29:21Z

(Awaiting review, as of this post)

…camp-2023

TongguangZhang

reviewed

TongguangZhang · 2025-01-09T13:54:06Z

modules/bootcamp/detect_landing_pad.py

+            if result:
+                bounding_boxes.append(box)


instead of skipping failed boxes, we acc wanna return an empty list - in data science, when part of a data point is corrupted, we skip the whole data point instead of returning partial data, in this case, 1 image is 1 data point, and the boxes are part of the image, so we skip the whole image if any of the boxes fail

TongguangZhang · 2025-01-09T13:55:27Z

modules/bootcamp/decision_simple_waypoint.py

+        on_waypoint = (
+            report.position.location_x - self.acceptance_radius
+            <= self.waypoint.location_x
+            <= report.position.location_x + self.acceptance_radius
+        ) and (
+            report.position.location_y - self.acceptance_radius
+            <= self.waypoint.location_y
+            <= report.position.location_y + self.acceptance_radius
+        )


acceptance radius we consider to be strictly <, not <=

TongguangZhang · 2025-01-09T13:56:18Z

modules/bootcamp/decision_simple_waypoint.py

+        on_waypoint = (
+            report.position.location_x - self.acceptance_radius
+            <= self.waypoint.location_x
+            <= report.position.location_x + self.acceptance_radius
+        ) and (
+            report.position.location_y - self.acceptance_radius
+            <= self.waypoint.location_y
+            <= report.position.location_y + self.acceptance_radius
+        )


this gives us a square acceptance, we actually want to use a circle, consider delta_x ^ 2 + delta_y ^ 2 < acceptance_radius ^ 2 instead

TongguangZhang · 2025-01-09T13:58:12Z

modules/bootcamp/decision_simple_waypoint.py

+            if on_waypoint:
+                command = commands.Command.create_land_command()


try to keep variables in scope, since on_waypoint is only used inside the if statement, you should only calculate it inside the if statement (it also avoids unnecessary calculations

TongguangZhang · 2025-01-09T13:59:06Z

modules/bootcamp/decision_waypoint_landing_pads.py

+            )
+
+            # Handle unexpected halts
+            if report.position != report.destination:


this is not how we check if the drone has reached a location - use the acceptance radius instead

TongguangZhang · 2025-01-09T14:04:27Z

modules/bootcamp/decision_waypoint_landing_pads.py

+        i = 0
+        winning_i = 0


it's better to store the landing pad object, not the index of the winning landing pad

TongguangZhang · 2025-01-09T14:04:43Z

modules/bootcamp/decision_waypoint_landing_pads.py

+        wp_x = self.waypoint.location_x
+        wp_y = self.waypoint.location_y


why would you use the waypoint location instead of the drone's position?

TongguangZhang · 2025-01-09T14:06:55Z

modules/bootcamp/decision_waypoint_landing_pads.py

+
+            sq_dist = (lp_x - wp_x) ** 2 + (lp_y - wp_y) ** 2
+
+            if (sq_dist < winning_dist) or (winning_dist == -1):


if you wanted to check if it was the first landing pad, I would suggest i == 1
however, we usually want to avoid this kind of hard coded edge case handling, instead, initialize winning_dist to infinity (search up how infinity is represented in python)

TongguangZhang · 2025-01-09T14:07:43Z

modules/bootcamp/decision_waypoint_landing_pads.py

+            same_loc = (
+                wp_x - self.acceptance_radius <= lp_x <= wp_x + self.acceptance_radius
+            ) and (wp_y - self.acceptance_radius <= lp_y <= wp_y + self.acceptance_radius)


why only - and not + this time?
but also, this is again a square, we want to use a circle instead

TongguangZhang · 2025-01-09T14:09:05Z

modules/bootcamp/decision_waypoint_landing_pads.py

+            if same_loc:
+                return i


why would you need to explicitly check if it is the same loc? if it was the same loc, wouldn't that landing pad also have the smallest distance and you'd return it anyways?

Correct, I had figured that if it were the same location then there was no point in continuing the loop -> it could just return right there.
However, if this rarely ever happens then it's not worth checking. I'll remove it, thx

TongguangZhang

reviewed

TongguangZhang · 2025-01-14T01:13:27Z

modules/bootcamp/decision_simple_waypoint.py

+            else:
+                command = commands.Command.create_set_relative_destination_command(
+                    self.waypoint.location_x, self.waypoint.location_y
+                )


note that this is relative position, not absolute, you can't just use the location of the waypoint (make sure you run your tests before requesting a rereview

TongguangZhang · 2025-01-14T01:19:41Z

modules/bootcamp/decision_waypoint_landing_pads.py

        # ============
        # ↓ BOOTCAMPERS MODIFY BELOW THIS COMMENT ↓
        # ============

-        # Do something based on the report and the state of this class...
+        # print("RUN")


it's ok to have print statements, but we want to delete commented out code, take a look at the style guide section on dead code

TongguangZhang · 2025-01-14T01:20:29Z

modules/bootcamp/decision_waypoint_landing_pads.py

-        # Default command
-        command = commands.Command.create_null_command()
-


don't modify code outside the comment guards - you deleted this and then copied it back down below

TongguangZhang · 2025-01-14T01:21:16Z

modules/bootcamp/decision_waypoint_landing_pads.py

+            print("HALTED AT:   ", report.position.location_x, report.position.location_y)
+            print(
+                "DESTINATION WAS:   ", report.destination.location_x, report.destination.location_y
+            )


it's worth taking a look at python string formatting - I'd recommend looking at f-string docs, as well as special characters such as -t (no need to change anything here, just smth to keep in mind)

TongguangZhang · 2025-01-14T01:21:55Z

modules/bootcamp/decision_waypoint_landing_pads.py

+            elif not self.approaching_wp:
+                command = commands.Command.create_set_relative_destination_command(
+                    self.waypoint.location_x, self.waypoint.location_y
+                )


incorrect, you need relative instead of absolute coords

TongguangZhang · 2025-01-14T01:26:15Z

modules/bootcamp/detect_landing_pad.py

+        predictions = self.__model.predict(
+            source=image, conf=0.7, device=self.__DEVICE, verbose=False
+        )
+        # print("PREDICTIONSSS:  ", prediction)


delete or uncomment

TongguangZhang · 2025-01-14T01:26:20Z

modules/bootcamp/detect_landing_pad.py


        # Get the Result object
-        prediction = ...
+        prediction = predictions[0]
+        # print("PREDICTION:  ", prediction)


delete or uncomment

TongguangZhang · 2025-01-14T01:26:27Z

modules/bootcamp/detect_landing_pad.py

+        # print("BOXES-CPU TYPE:  ", type(boxes_cpu))
+        # print("BOXES-CPU:  ", boxes_cpu)
+        # print("BOXES-CPU ND SHAPE", boxes_cpu.shape)


delete or uncomment

TongguangZhang · 2025-01-14T01:26:43Z

modules/bootcamp/detect_landing_pad.py

+            # print("RESULT:    ", result)
+            # print("BOX:    ", box)
+


delete or uncomment

TongguangZhang · 2025-01-14T01:27:10Z

modules/bootcamp/detect_landing_pad.py

+            if result:
+                bounding_boxes.append(box)
+            else:
+                return []


is this what the function is supposed to return? take another look at the return annotation, as well as your own code on line 136

Khalil-Hawari and others added 10 commits January 8, 2025 17:49

All files changed in Bootcamp

b2f22de

Delete detect_landing_pad.py

55ff8ef

Delete run_decision_example.py

d2580cf

Delete decision_example.py

e0b758b

Delete decision_simple_waypoint.py

a4ad320

Delete decision_waypoint_landing_pads.py

fa56623

Delete run_decision_simple_waypoint.py

d9ce896

Delete run_decision_waypoint_landing_pads.py

3db704b

Files modified in my bootcamp

b77cfa5

Merge branch 'main' of https://github.com/Khalil-Hawari/autonomy-boot…

3a12a34

…camp-2023

TongguangZhang reviewed Jan 9, 2025

View reviewed changes

Added changes from feedback

f3b0a4f

TongguangZhang reviewed Jan 14, 2025

View reviewed changes

Khalil-Hawari added 2 commits February 4, 2025 02:05

Updated detect landing pad

ab89d0b

Revised bootcamp code

c345cca

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Khalil Hawari Bootcamp #139

Khalil Hawari Bootcamp #139

Khalil-Hawari commented Jan 8, 2025

TongguangZhang left a comment

TongguangZhang Jan 9, 2025

TongguangZhang Jan 9, 2025

TongguangZhang Jan 9, 2025

TongguangZhang Jan 9, 2025

TongguangZhang Jan 9, 2025

TongguangZhang Jan 9, 2025

TongguangZhang Jan 9, 2025

TongguangZhang Jan 9, 2025

TongguangZhang Jan 9, 2025

TongguangZhang Jan 9, 2025

Khalil-Hawari Jan 11, 2025

TongguangZhang left a comment

TongguangZhang Jan 14, 2025

TongguangZhang Jan 14, 2025

TongguangZhang Jan 14, 2025

TongguangZhang Jan 14, 2025

TongguangZhang Jan 14, 2025

TongguangZhang Jan 14, 2025

TongguangZhang Jan 14, 2025

TongguangZhang Jan 14, 2025

TongguangZhang Jan 14, 2025

TongguangZhang Jan 14, 2025

		if on_waypoint:
		command = commands.Command.create_land_command()

		wp_x = self.waypoint.location_x
		wp_y = self.waypoint.location_y


		sq_dist = (lp_x - wp_x) 2 + (lp_y - wp_y) 2

		if (sq_dist < winning_dist) or (winning_dist == -1):

		# Default command
		command = commands.Command.create_null_command()

Khalil Hawari Bootcamp #139

Are you sure you want to change the base?

Khalil Hawari Bootcamp #139

Conversation

Khalil-Hawari commented Jan 8, 2025

TongguangZhang left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TongguangZhang left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment