Implement Land Obstacles - 278 #436
Conversation
run pip3 install in terminal first
…ent-land-obstacles-278
and optimized etl script
…/github.com/UBCSailbot/sailbot_workspace into user/SPDonaghy/implement-land-obstacles-278
sync with main branch
|
Needed to create the pkl file in the stable environment defined by the Dockerfile + package.xml, otherwise things went haywire with dependencies |
|
Okay, yeah this is done |
| multi_poly = MultiPolygon(geoms) | ||
| multi_poly = multi_poly.buffer(0) # repairs invalid geometry | ||
| # this is to prevent geometry the crosses -180 longitude from being distorted by transforms | ||
| with open(INLAND_POLYGON, "r") as f: |
There was a problem hiding this comment.
Could add error handling here
There was a problem hiding this comment.
Sure! which errors do you think we should plan for here?
There was a problem hiding this comment.
The file cannot open exception error
|
Will review the test cases later this week |
|
|
|
|
||
| self.collision_zone = collision_zone.buffer(COLLISION_ZONE_SAFETY_BUFFER, join_style=2) | ||
| else: |
There was a problem hiding this comment.
We only need to update land collision zones when we reach a global waypoint right? I wonder if its worth restructuring a bit so to avoid spending the effort to update all the land obstacles more often than we need? If its not a big efficiency change then I don't mind going with whatever is most logical and readable :)
There was a problem hiding this comment.
Yeah the decision of when to update will be up to the obstacle tracker. You update it directly using the update_collision_zone function or by updating the reference point. In practice, for the land obstacle, we should only be updating its collision zone through the update_reference_point function
There was a problem hiding this comment.
Clarification btw, at any time there will only be one land obstacle. A MultiPolygon containing 0 to n polygons
There was a problem hiding this comment.
Yeah the decision of when to update will be up to the obstacle tracker. You update it directly using the update_collision_zone function or by updating the reference point. In practice, for the land obstacle, we should only be updating its collision zone through the update_reference_point function
Clarification btw, at any time there will only be one land obstacle. A MultiPolygon containing 0 to n polygons
| def update_sailbot_data( | ||
| self, sailbot_position: HelperLatLon, sailbot_speed: Optional[float] = None | ||
| ) -> None: | ||
| """Updates Sailbot's position, and Sailbot's speed (if the caller is a Boat object). |
There was a problem hiding this comment.
Another good note! I was just about to ask why speed is optional
But now I'm a little confused from an architecture perspective why a Boat object (or also a land object?) would ask for the updated sailbot speed. I'd like to discuss this later, mostly for my own curiosity haha
There was a problem hiding this comment.
Boat objects need Sailbot's speed to calculate the size of the collision_zone for the moving ship. Land doesn't need Sailbot's speed, but still makes use of Sailbot's position when creating a state space like box in which it searches for intersections with land.
| if len(polygons) == 0: | ||
| return [] | ||
|
|
||
| elif len(polygons) == 1: | ||
| return [_latlon_polygon_to_xy_polygon(polygons[0])] | ||
|
|
||
| return list(map(_latlon_polygon_to_xy_polygon, polygons)) |
There was a problem hiding this comment.
What's the rationale for the different returns? I'm concerned the calling code will need to be more complicated to handle these cases
There was a problem hiding this comment.
The rationale is that based on the proximity to land and the shape of the coastline and state-space, we could be dealing with multiple polygons, a single polygon, or none so the function needs to consider those three possible cases. I get what you are saying, I'll just have the function do nothing if the list is empty or contains empty geometry. Only update_land_collision_zone will call this function. I am thinking of moving this function to moved it.coord_sytems.py thoughts?
Description
Verification