Bootcamp Pull #136
Bootcamp Pull #136
Conversation
| for i in range(0, boxes_cpu.shape[0]): | ||
| bounding_boxes.append(bounding_box.BoundingBox.create(boxes_cpu[i])[1]) | ||
|
|
||
| return bounding_boxes, image_annotated |
There was a problem hiding this comment.
take a closer look at the create() function, there's a reason why it returns a tuple, take a look at what the first element of the tuple represents
|
|
||
| # Detach the xyxy boxes to make a copy, | ||
| # move the copy into CPU space, | ||
| # and convert to a numpy array | ||
| boxes_cpu = ... | ||
| boxes_cpu = boxes_xyxy.cpu().numpy() |
There was a problem hiding this comment.
make sure to call detach(), this removes it from the computational graph
basically, these tensors have gradient tracking, which is used for back propagation in model training, but since we're using the inference, we don't need the gradients, and they take up a lot of extra memory
| @@ -98,22 +98,22 @@ def run(self, image: np.ndarray) -> "tuple[list[bounding_box.BoundingBox], np.nd | |||
| # * conf | |||
| # * device | |||
| # * verbose | |||
| predictions = ... | |||
| predictions = self.__model.predict(image, conf=0.25, device=self.__DEVICE, verbose=True) | |||
There was a problem hiding this comment.
0.25 might be a little too low here
| def get_y_difference(initial: location.Location, final: location.Location): | ||
| return (final.location_y - initial.location_y) | ||
|
|
||
| def get_distance_squared(diff_x, diff_y): |
There was a problem hiding this comment.
let's use **2 to make it more readable that ur doing the square
| if (report.status == drone_status.DroneStatus.MOVING): | ||
| command = commands.Command.create_null_command() |
There was a problem hiding this comment.
do you need to do this? if you remove the if statement and the drone is moving, wouldn't it return a null command by default?
There was a problem hiding this comment.
If I remove the if statement the drone gets spammed with set_destination commands since most of the time its quite a ways from the landing pad. I might be missing something but thats the reason that I added it
| def get_y_difference(initial: location.Location, final: location.Location): | ||
| return (final.location_y - initial.location_y) | ||
|
|
||
| def get_distance_squared(diff_x, diff_y): |
There was a problem hiding this comment.
same as above, use **2
| return (diff_x*diff_x + diff_y*diff_y) | ||
|
|
||
| def get_closest_landing_pad(): | ||
| min_distance = 1e20 |
There was a problem hiding this comment.
instead of using a very large number, try looking into how u represent infinity in python
|
|
||
| def get_closest_landing_pad(): | ||
| min_distance = 1e20 | ||
| closest_landing_pad = None |
There was a problem hiding this comment.
make sure you handle the case where the landing pad is still none (no landing pads in the list)
| if (report.status == drone_status.DroneStatus.MOVING): | ||
| command = commands.Command.create_null_command() |
There was a problem hiding this comment.
same as above, is this necessary?
There was a problem hiding this comment.
If I remove the if statement the drone gets spammed with set_destination commands since most of the time its quite a ways from the landing pad. I might be missing something but thats the reason that I added it
| if result: | ||
| bounding_boxes.append(box) # do nothing if a bounding box isn't found. |
There was a problem hiding this comment.
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
| if report.status == drone_status.DroneStatus.MOVING: | ||
| command = commands.Command.create_null_command() |
There was a problem hiding this comment.
it's ok to spam the drone with commands, but it's also ok to do what you did here, you don't have to change anything
|
|
||
| if report.status == drone_status.DroneStatus.MOVING: | ||
| command = commands.Command.create_null_command() | ||
| elif self.get_distance_squared(x_difference, y_difference) >= ( |
There was a problem hiding this comment.
are you sure that you don't need to check that the drone is halted? remember that moving and halted are not the only 2 possible statuses, to be consistent with above, you probably don't want to keep sending land commands if the drone is already landed
| if report.status == drone_status.DroneStatus.MOVING: | ||
| command = commands.Command.create_null_command() | ||
| elif self.get_distance_squared(x_difference, y_difference) >= ( | ||
| self.acceptance_radius * self.acceptance_radius |
| # No landing pads found => land at current location. | ||
| self.waypoint = result if result is not None else report.position |
There was a problem hiding this comment.
this is incorrect, the instructions don't say to land at the waypoint if no landing pads are found, and you shouldn't make that assumption
since it doesn't tell you how to handle the case, you should return the default command if there are no landing pads and let the rest of the code handle it (consider in a larger system, we might decide to fly around and search for landing pads if none are found, not land)
| # No landing pads found => land at current location. | ||
| self.waypoint = result if result is not None else report.position |
There was a problem hiding this comment.
it's a little bit confusing to overwrite self.waypoint with the landing pad's location, I would store a self.landing_pad variable, but it's fine to do it this way too, you don't need to change anything here
|
|
||
| if report.status == drone_status.DroneStatus.MOVING: | ||
| command = commands.Command.create_null_command() | ||
| elif self.get_distance_squared(x_difference, y_difference) >= (self.acceptance_radius**2): |
There was a problem hiding this comment.
same as above, check drone status is halted
|
|
||
| if report.status in { | ||
| drone_status.DroneStatus.MOVING, | ||
| report.status == drone_status.DroneStatus.LANDED, |
There was a problem hiding this comment.
this is incorrect, remove the report.status == - this just evaluates to True or False, so the whole statement becomes report.status in {MOVING, True/False}, which is not what you want
| """ | ||
| return diff_x**2 + diff_y**2 | ||
|
|
||
| def get_closest_landing_pad( |
There was a problem hiding this comment.
this can also be static - the functions you call here are static methods, which don't require an instance of the class (self) to call
| # If self.waypoint is None (also handles the case when no landing pad is found) default null command is sent | ||
| if ( | ||
| report.status | ||
| in {drone_status.DroneStatus.MOVING, report.status == drone_status.DroneStatus.LANDED} |
No description provided.