bugGNU Octave - Bugs: bug #65291, [octave forge] (mapping)...

 
 

bug #65291: [octave forge] (mapping) degrees2dms produces incorrect results

Submitter:  Goran Gasparovic <gasparovic>
Submitted:  Sun 11 Feb 2024 01:14:53 PM UTC
   
 
Category:  Octave Forge Package Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Incorrect Result
Status:  Confirmed Assigned to:  None
Originator Name:  Goran Gasparovic Open/Closed:  * Open
Release:  * other Operating System:  * Any
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Tue 13 Feb 2024 09:00:18 PM UTC, comment #8: 

OK I'll set status back to just "confirmed".

These functions (degrees2dms, dms2degrees, dm2degrees, dms2degrees) haven't been written by me.  While their code looks quite straightforward, corner cases like losing seconds or minutes easily fall victim to double (im)precision that isn't catched properly.
I'm also a bit surprised that this bug pops up only now.

As an aside:
Degrees-minutes-seconds display output like [1, 11, 60] looks a bit wonky as it suggests that a minute can be subdivided in 61 ... yeah, what? 'intervals'? rather than 60 seconds (0..60 rather than 0..59). OTOH display output like in comment #2, where output precision is much higher, is accurate. It is the display rounding that's hard to get right.
So I can understand e.g. Matlab in emitting such results - it's a lot easier to not have to worry about carry-over when rounding to display precision. But in most practical use (e.g., navigating at sea) it would be frowned upon and most navigation SW I know either doesn't accept such input or morphs it into "proper" display rounding.

From comment #2:

> when rounded to nearest double is [1, 11, 59.9999999999998436805981327779591083526611328125]

Rounding to display precision (IIUC different than "double precision") should give [1, 12, 0].  On the surface of the Earth the inaccuracy would be in the order of 3^(-10) m:

>> (60 - 59.9999999999998436805981327779591083526611328125) * 1852
ans = 2.895035322580952e-10

(one minute ~ a nautical mile = 1852 m)

I might give fixing it another try later on, but I have little time to devote to it, sorry. Hopefully someone beats me to it.

Philip Nienhuis <philipnienhuis>
Group Member
Tue 13 Feb 2024 06:38:40 PM UTC, comment #7: 


a = single(0:(2 .^ 23 - 1)) .';
b = 1 + a .* eps('single');
positive_normal_mismatches = 0;
positive_normal_total = 0;
for p = (-126):127
d = pow2(p) .* b;
positive_normal_mismatches = positive_normal_mismatches + sum(any(d2dms(d) ~= degrees2dms(d), 2));
positive_normal_total = positive_normal_total + numel(d);
end
positive_normal_mismatches
positive_normal_mismatches =
     0
positive_normal_matches = positive_normal_total - positive_normal_mismatches
positive_normal_matches =
     2.130706432000000e+09
d = pow2(-149) .* a;
positive_zero_and_subnormal_mismatches = sum(any(d2dms(d) ~= degrees2dms(d),2))
positive_zero_and_subnormal_mismatches =
     0
positive_zero_and_subnormal_matches = numel(d) - positive_zero_and_subnormal_mismatches
positive_zero_and_subnormal_matches =
     8388608


Nicholas Jankowski <nrjank>
Group Member
Tue 13 Feb 2024 05:44:03 PM UTC, comment #6: 

It would be interesting to see the output of MATLAB for the following which I expect will run in less than 5 minutes:


format long

function dms = d2dms (d)
  df = fix(d);
  m = (d - df) .* 60;
  mf = fix(m);
  s = (m - mf) .* 60;
  dms = [df, mf, s];
end

a = single(0:(2 .^ 23 - 1)) .';
b = 1 + a .* eps('single');

positive_normal_mismatches = 0;
positive_normal_total = 0;
for p = (-126):127
  d = pow2(p) .* b;
  positive_normal_mismatches = positive_normal_mismatches + sum(any(d2dms(d) != degrees2dms(d), 2));
  positive_normal_total = positive_normal_total + numel(d);
end
positive_normal_mismatches
positive_normal_matches = positive_normal_total - positive_normal_mismatches

d = pow2(-149) .* a;
positive_zero_and_subnormal_mismatches = sum(any(d2dms(d) != degrees2dms(d), 2))
positive_zero_and_subnormal_matches = numel(d) - positive_zero_and_subnormal_mismatches


Anonymous
Tue 13 Feb 2024 01:50:47 PM UTC, comment #5: 

yes, seems like we're still losing the minute:

matlab:

>> degrees2dms(2-eps())
ans =
   1.000000000000000  59.000000000000000  59.999999999999147


the BIST was likely just set without considering float or display error.

again, matlab gives:


>> degrees2dms(10+0.5/60)
ans =
   10.0000         0   30.0000

>> format long
>> ans
ans =
  10.000000000000000                   0  29.999999999998295

>> [10 0 30] - ans
ans =
   1.0e-11 *
                   0                   0   0.170530256582424



Nicholas Jankowski <nrjank>
Group Member
Tue 13 Feb 2024 06:49:43 AM UTC, comment #4: 

With the proposed fix from comment #2, degrees2dms(2 - eps()) returns [1, 60, 59.9999999999990905052982270717620849609375] for me, which is off by 1 minute or so.

If a well-defined specification for this function can be developed, it might be feasible for proposed implementations to be exhaustively tested on inputs of class single.

Some of the current test cases seem strange.  For instance, the exact result for degrees2dms(10 + 0.5 / 60) would be [10, 0, 29.9999999999982946974341757595539093017578125] which is exactly representable, but a test requires it to return exactly [10, 0, 30].

Anonymous
Mon 12 Feb 2024 10:53:19 PM UTC, comment #3: 

In Octave, 1.2 means the nearest double which is actually 1.1999999999999999555910790149937383830547332763671875.  The exact result for this input would be [1, 11, 59.999999999999840127884453977458178997039794921875] which when rounded to nearest double is [1, 11, 59.9999999999998436805981327779591083526611328125].

Anonymous
Mon 12 Feb 2024 05:40:03 PM UTC, comment #2: 

Please try with attached degrees2dms.m and report back.

Put it somewhere in your path before the mapping package paths, or try to swap it with the one in the mapping package. You can find that one using:
which degrees2dms
in Octave's command window.
If it works for you I'll push a fix.


The original bug got fixed by just adding or subtracting eps.
But Octave's degrees2dms, like Matlab's, also accepts logical and character input although the Matlab docs only mention "real-valued input". So the fix needed some extra provisioning.
It all feels a bit like a kludge but it works for me.

Status set to "patch submitted".

(file #55687)

Philip Nienhuis <philipnienhuis>
Group Member
Sun 11 Feb 2024 03:48:58 PM UTC, comment #1: 

checking in matlab 2023b:


>> degrees2dms(1.2)

ans =

    1.0000   11.0000   60.0000

>> ans - [1 11 60]

ans =

   1.0e-12 *

         0         0   -0.2132


vs octave (9.0.91), mapping 1.4.2 (which loads matgeom 1.2.3 and geometry 4.0.0):


>> degrees2dms(1.2)
ans =

    1   11    0


>> ans - [1 11 0]
ans =

   0   0   0


marking as confirmed, cc'ing pkg maintainer.

Nicholas Jankowski <nrjank>
Group Member
Sun 11 Feb 2024 01:14:53 PM UTC, original submission:  

Bug report, octave-8.4.0-w64.7z, "Intel(R) Core(TM) i3-5005U CPU @ 2.00GHz   2.00 GHz".


-----
>> pkg load mapping
warning: function (...) octave-8.4.0-w64\octave-8.4.0-w64\mingw64\share\octave\packages\matgeom-1.2.3\geom2d\normalize.m shadows a core library function (...)
>> degrees2dms (1.2)
ans =
    1   11    0
-----


The answer should be [1 12 0].

Goran Gasparovic <gasparovic>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #55687:  degrees2dms.m added by philipnienhuis (4KiB - text/plain)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by nrjank (Posted a comment)
  • -email is unavailable- added by nrjank
  • -email is unavailable- added by gasparovic (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only group members can vote.

     

    Follow 10 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2024-02-13 philipnienhuis StatusPatch Submitted Confirmed
        Assigned tophilipnienhuis None
        Operating SystemMicrosoft Windows Any
    2024-02-12 philipnienhuis Attached File- Added degrees2dms.m, #55687
        StatusConfirmed Patch Submitted
        Assigned toNone philipnienhuis
    2024-02-11 nrjank StatusNone Confirmed
        Release8.4.0 other
        SummaryPkg mapping: degrees2dms [octave forge] (mapping) degrees2dms produces incorrect results
        Carbon-Copy- Added -email is unavailable-

    Back to the top

    Powered by Savane 3.12.
    Corresponding source code