search

seandepagnier / weather_routing_pi

weather routing plugin for opencpn
GNU General Public License v3.0
47 stars 35 forks source link

Log polar error code, Log polar parsing warnings + polar parsing logic when value is zero #302

Open sebastien-rosset opened 4 months ago

sebastien-rosset commented 4 months ago

Problem statement

  1. When the polar contains a zero value, a parsing warning is created, but no log is generated in opencpn.log. This makes it more difficult to troubleshoot problems.
  2. When the above issue occurs, the first element of the Polar::wind_speed is set to NaN. It seems it should be set to zero.
  3. When the first element of the Polar::wind_speed is set to NaN, the Polar::Speed function may return Nan, and a message is printed to stdout. It should probably have been logged to opencpn.log, which makes it easier to troubleshoot.

Example print statements to stdout when polar contains NaN:

polar failed bad! 18.487259 7.997830 21.487259 nan
polar failed bad! 18.487259 7.997830 24.487259 nan
polar failed bad! 18.487259 7.997830 27.487259 nan
polar failed bad! 18.487259 7.997830 30.487259 nan
polar failed bad! 18.487259 7.997830 33.487259 nan
polar failed bad! 18.487259 7.997830 36.487259 nan
polar failed bad! 18.487259 7.997830 39.487259 nan

Changes in this PR

  1. When the plugin opens a polar and warning-level parse issues are found, log the parse warnings to opencpn.log. This is useful for troubleshooting route calculation issues. For example: 

    11:32:18.975 MESSAGE Polar.cpp:331 Test-TWS-0-20+60.pol line 2: Warning: 0 values found in polar.
These measurements will be interpolated.
To specify interpolated, leave blank values.
To specify course as 'invalid' use 0.0 rather than 0

  2. When a polar contains the 0 value, initialize the first element of the Polar::wind_speed array with the 0 value instead of NaN.

    1. Perhaps setting the first element of the Polar::wind_speed array to NaN was intentional?? See analysis below.
    2. When a route is calculated based on polars that contain the zero value, this ends up causing the route calculation to fail.

  3. When route calculation fails, use wxLogMessage instead of printf to report the problem. Improve the message to make it easier to troubleshoot the issue.

With this PR, the print statements are now written to opencpn.log:

10:58:31.752 MESSAGE ocpn_frame.cpp:5590 LOGBOOK:  2024-07-07 17:58:31 UTC  DR Lat   33.35800 Lon  -79.28200
10:58:59.467 WARNING RouteMap.cpp:673 polar failed. V: 18.487259 VW: 7.997830 B: 21.487259 VB: nan H: 3.000000. Error: 6
10:58:59.467 WARNING RouteMap.cpp:673 polar failed. V: 18.487259 VW: 7.997830 B: 24.487259 VB: nan H: 6.000000. Error: 6
10:58:59.467 WARNING RouteMap.cpp:673 polar failed. V: 18.487259 VW: 7.997830 B: 27.487259 VB: nan H: 9.000000. Error: 6
10:58:59.467 WARNING RouteMap.cpp:673 polar failed. V: 18.487259 VW: 7.997830 B: 30.487259 VB: nan H: 12.000000. Error: 6
10:58:59.467 WARNING RouteMap.cpp:673 polar failed. V: 18.487259 VW: 7.997830 B: 33.487259 VB: nan H: 15.000000. Error: 6

Analysis

When a polar is loaded, some warnings are generated with PARSE_WARNING, but these warnings are not necessarily displayed anywhere. The warnings are not logged in opencpn.log either. This can make route failure harder to understand.

In particular, when a polar file contains a 0 value, Polar::Open adds the NaN value to the first entry in the Polar::wind_speeds array. For example, given the following polar:

twa/tws;6;8;10;12;14;16;20;60
0;0;0;0;0;0;0;0;0
50;5.4;6.5;7.1;7.4;7.5;7.6;7.8;0

The Polar::wind_speeds array is populated as [NaN, 5.4, 5.6, 6, 6.1, 6, 5.6, 5.0, 4.3] Relevant code: https://github.com/seandepagnier/weather_routing_pi/blob/97f2a08dfb06b4847d9790b11b6155d3bca5efd2/src/Polar.cpp#L299-L315

The NaN value in the wind_speeds array ends up causing Polar::Speed() to return VB=NaN at https://github.com/seandepagnier/weather_routing_pi/blob/97f2a08dfb06b4847d9790b11b6155d3bca5efd2/src/Polar.cpp#L537

seandepagnier commented 4 months ago

it was intentional... 0 means the boat is not moving. NaN means we do not know the boat speed.. but maybe it is still broken?

sebastien-rosset commented 4 months ago

it was intentional... 0 means the boat is not moving. NaN means we do not know the boat speed.. but maybe it is still broken?

Thanks, the history is helpful. I will check again and get back.

sebastien-rosset commented 4 months ago

Based on this code:

                  PARSE_WARNING(_("Warning: 0 values found in polar.\n"
                                        "These measurements will be interpolated.\n"
                                        "To specify interpolated, leave blank values.\n"
                                        "To specify course as 'invalid' use 0.0 rather than 0\n"));

and:

** blank = Interpolate from nearby values, if at least one set of
values of tws/twd are entered in the column/row.
*** 0 = Not recognized by the program, will remove these it edited.
Sometimes a polar will contain 0's. They should be removed.
*** 0.0 = Specifies invalid (cannot be used). Boat won't move and can't
go there.

and:

<property name="label">Leave any cell blank to automatically interpolate from nearby values.  
Use a value of 0.0 to specify invalid (cannot be used)&#x0A;
View the polar plot in the boat dialog while editing the polar.</property>

There are four possible values in a polar:

Value Description
> 0 Normal boat speed value
0.0 Boat speed is considered "invalid"
empty Value will be interpolated using neighboring cells
0 The first time "0" is found in the polar, the cell value is set to NaN. This means the value will be interpolated using neighboring cells. Subsequent "0" values in the polar are stored as 0.0 i.e. boat speed is zero.

Values are interpolated only if either:

  1. There are lower and higher wind speed values.
  2. There are lower and higher angle values.

In other cases, the polar value is not interpolated and remains NaN.

Once the polar values have been loaded and the plugin calculates a route, the value NaN is considered invalid. I.e. the plugin will not use this path.

I'm wondering if this logic could be simplified.

  1. Do we really need to treat "0" and "0.0" differently? It's not intuitive.
  2. Why do we need to treat the first "0" value as a special case (converted to NaN) , whereas all other "0" string values are converted to 0.0 numerical values.
  3. Maybe I'm missing something, but what is the rationale for the "0" string value logic? This may be naive, but if I see the value "0", I think that means the boat is not moving for that wind speed and angle; for example, if the boat is directly upwind (TWA is 0) then it makes sense that the boat speed is 0.
    1. If the "0" value is in the top-left cell, the value is not interpolated, hence it will be set to NaN. But if the wind angle is zero, it makes sense that the boat speed is 0.0, i.e. the boat speed is not "unknown".
    2. If "0" is present in the middle of the polar file, then the 0 value is replaced with the interpolated value based on the neighboring cells. And all other "0" string values will be set to the 0.0 numerical value.

For case 3.i, the code hits this path: https://github.com/seandepagnier/weather_routing_pi/pull/302/files#diff-e6e0fa7dd681f90e3e13a71ae1102a4bb0c52ae1173eb1dad0865264df6ed903R673 and prints failure messages such as:

polar failed bad! 18.487259 7.997830 21.487259 nan
polar failed bad! 18.487259 7.997830 24.487259 nan
polar failed bad! 18.487259 7.997830 27.487259 nan
polar failed bad! 18.487259 7.997830 30.487259 nan

It the logic remains as is, then perhaps the values in .opencpn/plugins/weather_routing/polars/Example/Test-TWS-0-20+60.pol for the first row should be changed from "0" to "0.0".