Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

sebastien-rosset
Copy link
Contributor

@sebastien-rosset sebastien-rosset commented Jul 5, 2024

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
  1. 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.
  2. 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:

double s = NAN;
if((token = strtok_polar(NULL, &saveptr))) {
if(!strcmp(token, "0") && !warn_zeros) {
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"));
warn_zeros = true;
} else if(*token) {
s = strtod(token, 0);
if(s < .05)
s = 0;
}
} else
PARSE_WARNING(_("Too few measurements."));
wind_speeds[VWi].orig_speeds.push_back(s);

The NaN value in the wind_speeds array ends up causing Polar::Speed() to return VB=NaN at

double VB11 = ws1.speeds[W1i], VB12 = ws1.speeds[W2i];

src/Polar.cpp Outdated
@@ -304,6 +304,7 @@ bool Polar::Open(const wxString &filename, wxString &message)
"To specify interpolated, leave blank values.\n"
"To specify course as 'invalid' use 0.0 rather than 0\n"));
warn_zeros = true;
s = 0;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was it intentional that s was set to NaN for the first element?

@sebastien-rosset sebastien-rosset marked this pull request as ready for review July 5, 2024 20:03
@sebastien-rosset sebastien-rosset changed the title Log polar error code Log polar error code, Log polar parsing warnings + polar parsing logic when value is zero Jul 5, 2024
@seandepagnier
Copy link
Owner

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
Copy link
Contributor Author

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
Copy link
Contributor Author

sebastien-rosset commented Jul 7, 2024

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".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants