Rework wind cost function in ompl objectives by AMaharaj16 · Pull Request #817 · UBCSailbot/sailbot_workspace

AMaharaj16 · 2026-02-21T22:16:24Z

The function wind_direction_cost() in ompl_objectives.py now correctly returns 1.0 when the segment heading lies within NO_GO_ZONE radians (45 degrees) of directly upwind (0) or directly downwind (π), and applies sin⁸⁰(2θ) otherwise. Also replaced old test with new fully passing test suite for this improved implementation in test_ompl_objectives.py.

The wind cost function looks like this:

… docstring to the new functionality.

…asses even if constant values are changed.

raghumanimehta

Please take a look at the longest comment I left on this review before anything else.

raghumanimehta · 2026-02-27T00:04:34Z

+        """Computes a wind alignment cost based on the absolute angle θ between the segment
+        bearing and the true wind direction.
+
+        1) If θ ≤ NO_GO_ZONE or θ ≥ π − NO_GO_ZONE (i.e., within 45 degrees of directly upwind or


what is theta? Please define what you mean by theta here.

Directly above, it says "Computes a wind alignment cost based on the absolute angle θ between the segment bearing and the true wind direction." Is this not sufficient?

it's sufficient. can you extract that definition and write it like: "theta: ..." ?

raghumanimehta · 2026-02-27T00:05:52Z

-            return UPWIND_COST_MULTIPLIER * cos_angle
-        else:
-            return DOWNWIND_COST_MULTIPLIER * abs(cos_angle)
+        # NO-GO ZONE


This comment, imo, is not required. The reader can understand what this is for by reading the if statement and the docs about the funciton.

raghumanimehta · 2026-02-27T01:15:24Z

@@ -107,12 +115,13 @@ def wind_direction_cost(s1: cs.XY, s2: cs.XY, tw_direction_rad: float) -> float:
        """
        segment_true_bearing_rad = cs.get_path_segment_true_bearing(s1, s2, rad=True)
        tw_angle_rad = abs(wcs.get_true_wind_angle(segment_true_bearing_rad, tw_direction_rad))


NOT YOUR FAULT.

While reviewing the PR, I realized that the conventions used in PATH are not consistent. Can you please look into it? Or let me know if you want me to look into it.
This funciton's example said that 0 degrees is from the bow. Other parts of the codebase are using 0 degrees as to the bow. This inconsistency is not nice. We should fix it asap. I am not going to review this PR for the time being.

If you do look into this issue, please also change the naming of the winds to our new standard. Eg. tw_speed_kmph_bcrather than wind_speed. There are also some cases where we name it as tw_speed_kmph, however notice the coordinate system is missing. Change these .i.e append bc or gc accordingly .

…UBCSailbot/sailbot_workspace into AMaharaj16/wind-cost-rework-793

- reduced True Wind/Apparent Wind to tw/aw - added gc and bc to variables/constant/fields accordingly

AMaharaj16 · 2026-04-03T19:59:30Z

The main inconsistency I have found is that global_to_boat_coordinate and boat_to_global_coordinate both assume headwind is 180 degrees, when it should be 0 degrees. This is because we want our wind convention to align with the idea that wind direction is where the wind is coming from, not where the wind is going to.

For example:

0 degrees in boat coords -> headwind or wind travelling from front to back of boat.
0 degrees in global coords -> wind travelling from north to south.

The rest of wind_coord_systems.py is fine. For global_to_boat_coordinate and boat_to_global_coordinate, I will adjust these functions, their tests, and wherever they get called.

…called anywhere in local pathfinding.

…ied to direction/heading.

AMaharaj16 · 2026-04-03T22:58:56Z

-        wind_direction (float): The direction of the wind in radians (-pi, pi]
-        wind_speed (float): The speed of the wind in m/s
+        tw_direction_rad_gc (float): The direction of wind in global coordinate radians (-pi, pi]
+        tw_speed_kmph (float): The speed of the true wind in km/h


want to emphasize here that I converted m/s to km/h. need clarification on impacts this may have, if any.

raghumanimehta

Thanks for the PR! One of the function changes is incorrect and there are some naming inconsistencies that need to be ironed out.
Also the documentation for wind system was not updated (WindReference.md), please do that and also add brief documentation in source code in wind_coord_systems.py.

raghumanimehta · 2026-04-03T22:28:58Z



-def boat_to_global_coordinate(boat_heading: float, wind_direction: float):
+def boat_to_global_coordinate(boat_heading_deg_gc: float, aw_direction_deg_bc: float):


Please rename aw_direction_deg_bc to aw_dir_deg_bc like it is in other files.

raghumanimehta · 2026-04-03T23:26:06Z


-TRUE_WIND_SPEEDS = [0.0, 11.1, 14.8, 18.5, 22.2, 25.9, 29.6, 37.0, 55.0, 75.0]
-SAILING_ANGLES = [0, 45, 50, 60, 75, 90, 110, 120, 135, 150, 180]
+TW_SPEEDS = [0.0, 11.1, 14.8, 18.5, 22.2, 25.9, 29.6, 37.0, 55.0, 75.0]


Add unit here please.

raghumanimehta · 2026-04-03T23:26:53Z

-TRUE_WIND_SPEEDS = [0.0, 11.1, 14.8, 18.5, 22.2, 25.9, 29.6, 37.0, 55.0, 75.0]
-SAILING_ANGLES = [0, 45, 50, 60, 75, 90, 110, 120, 135, 150, 180]
+TW_SPEEDS = [0.0, 11.1, 14.8, 18.5, 22.2, 25.9, 29.6, 37.0, 55.0, 75.0]
+SAILING_ANGLES_DEG = [0, 45, 50, 60, 75, 90, 110, 120, 135, 150, 180]


What exactly is SAILING_ANGLES?

raghumanimehta · 2026-04-10T00:40:46Z

+        """Computes a wind alignment cost based on the absolute angle θ between the segment
+        bearing and the true wind direction.
+
+        1) If θ ≤ NO_GO_ZONE or θ ≥ π − NO_GO_ZONE (i.e., within 45 degrees of directly upwind or


it's sufficient. can you extract that definition and write it like: "theta: ..." ?

raghumanimehta · 2026-04-10T00:42:52Z



-def get_true_wind_angle(boat_heading_rad: float, tw_dir_rad: float) -> float:
+def get_true_wind_angle(boat_heading_rad_gc: float, tw_dir_rad_gc: float) -> float:


Can you add a flag like https://github.com/UBCSailbot/sailbot_workspace/pull/817/changes#diff-0499bea8455563b5f619aca7cfeb228edb6c5b0b091345d2f45ebb66cccdcd5cL120 here?

raghumanimehta · 2026-04-10T00:44:10Z


    bw_speed_kmph = boat_speed_kmph
-    bw_dir_rad = math.radians(cs.bound_to_180(boat_heading_deg + 180.0))
+    bw_dir_rad = math.radians(cs.bound_to_180(boat_heading_deg_gc + 180.0))


for consistency, you should add gc as a suffix here as well

raghumanimehta · 2026-04-10T00:44:42Z

        calculations will break down.
    """
-    tw_dir_rad = math.radians(tw_dir_deg)
+    tw_dir_rad = math.radians(tw_dir_deg_gc)


for consistency, you should add gc as a suffix here as well

raghumanimehta · 2026-04-10T00:44:52Z

-    bw_dir_rad = math.radians(cs.bound_to_180(boat_heading_deg + 180))
-    aw_east_kmph = aw_speed_kmph * math.sin(aw_dir_rad)
-    aw_north_kmph = aw_speed_kmph * math.cos(aw_dir_rad)
+    bw_dir_rad = math.radians(cs.bound_to_180(boat_heading_deg_gc + 180))


for consistency, you should add gc as a suffix here as well

raghumanimehta · 2026-04-10T01:01:00Z

        20
    """
-    return cs.bound_to_180(boat_heading + wind_direction + 180.0)
+    return cs.bound_to_180(boat_heading_deg_gc + aw_direction_deg_bc)


this is wrong. the function before this change was in fact correct. Your tests are wrong as well. Here is why consider the example:

boat heading gc = -45.0 deg
aw_dir_bc = 0.0 deg

If you draw this out on a paper, you should see that this points to 135.0 deg which is exactly what the previous function was doing. Your change will return 0.0 deg which is wrong.

AMaharaj16 added 5 commits February 21, 2026 11:53

Implementation changed. Old tests removed.

2b0902d

Small bug in implementation. Some of testing completed.

f48c7a0

Robust, fully passing test suite. PR ready now.

cd822c1

Merge branch 'main' into AMaharaj16/wind-cost-rework-793

983fce3

Flipped some things to make the function read better, and updated the…

cfcbd13

… docstring to the new functionality.

AMaharaj16 requested a review from raghumanimehta as a code owner February 21, 2026 22:16

Refactored testing to include WIND_COST_SIN_EXPONENT constant so it p…

0a5463b

…asses even if constant values are changed.

raghumanimehta requested changes Feb 27, 2026

View reviewed changes

raghumanimehta and others added 5 commits February 26, 2026 17:16

Merge branch 'main' into AMaharaj16/wind-cost-rework-793

ee01c43

Merge branch 'main' into AMaharaj16/wind-cost-rework-793

bcc6ff1

Merge branch 'AMaharaj16/wind-cost-rework-793' of https://github.com/…

409b013

…UBCSailbot/sailbot_workspace into AMaharaj16/wind-cost-rework-793

Adjusted naming conventions in wind_coord_systems and ompl_objectives:

2c99e12

- reduced True Wind/Apparent Wind to tw/aw - added gc and bc to variables/constant/fields accordingly

Extended changes to testing files.

d85c77b

AMaharaj16 added 2 commits April 3, 2026 13:09

Adjusted functions and their testing suites. They don't appear to be …

6718f00

…called anywhere in local pathfinding.

Fixed test_mock_wind_sensor_pipeline.

5016fc8

AMaharaj16 requested review from MilcToast, ethanhu912, jb3982 and raghumanimehta April 3, 2026 20:22

Removed bc and gc from speed variables. Didn't realize they only appl…

0fc728f

…ied to direction/heading.

AMaharaj16 commented Apr 3, 2026

View reviewed changes

get_true_wind_angle converts true wind into boat coords.

5c2500d

raghumanimehta added the path Pathfinding team label Apr 10, 2026

raghumanimehta requested changes Apr 10, 2026

View reviewed changes



		def boat_to_global_coordinate(boat_heading: float, wind_direction: float):
		def boat_to_global_coordinate(boat_heading_deg_gc: float, aw_direction_deg_bc: float):



		def get_true_wind_angle(boat_heading_rad: float, tw_dir_rad: float) -> float:
		def get_true_wind_angle(boat_heading_rad_gc: float, tw_dir_rad_gc: float) -> float:

Conversation

AMaharaj16 commented Feb 21, 2026

Uh oh!

raghumanimehta left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AMaharaj16 commented Apr 3, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

raghumanimehta left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants