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

 
 

bug #62330: [octave forge] (mapping) degrees2dm fails on NaN input

Submitter:  CarnĂ« Draug <carandraug>
Submitted:  Mon 18 Apr 2022 11:53:18 AM UTC
 
Category:  Octave Forge Package Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Unexpected Error or Warning
Status:  Fixed Assigned to:  philipnienhuis
Originator Name:  Brendon Higgins Open/Closed:  * Closed
Release:  * other Operating System:  * Any
* Mandatory Fields

Add a New Comment (Rich Markup)
   

Comment Type & Canned Response:
       

 

Fri 15 Jul 2022 02:58:42 PM UTC, comment #4: 

After applying the patch, I get this error at L.90 (nn = ! isnan (dm(:, 1));):

## Pressing <F9> after selecting L.90:
debug> nn = ! isnan (dm(:, 1))
nn = 0
debug>   ## Pressing 'Step' button
error: invalid conversion from NaN to logical
error: called from
    degrees2dm at line 90 column 10

The code looks OK to me and executes correctly as seen above.

Hmmmm, removing the space between '!' and 'isnan' leads to correct execution of the code.

And after restarting Octave the original patched code (w space between ! and isnan) also executes fine.

I pushed the cset here:
http://hg.code.sf.net/p/octave/mapping/rev/9ef7a6ac0656
and added a BIST for that case here:
http://hg.code.sf.net/p/octave/mapping/rev/a9e52842884c

Thanks for the patch and other actions.
Closing report.

Philip Nienhuis <philipnienhuis>
Project Member
Tue 17 May 2022 08:49:39 PM UTC, comment #3: 

Thanks Nicholas.

I'll have a look at the patch but it might be mid-July before I have time for it.

Philip Nienhuis <philipnienhuis>
Project Member
Tue 17 May 2022 08:26:16 PM UTC, comment #2: 

unreviewed, but after converting line endings I saved his degrees2dm against the latest mapping pkg source and produced the attached hg patch which amounts to:

@@ -87,7 +87,9 @@
     ## leads to some precision errors.

     ## remove sign from any minute where its degree part is non-zero
-    nnzd = logical (dm(:,1));
+    nn = ! isnan (dm(:,1));
+    nnzd = false (rows (dm), 1);
+    nnzd(nn) = logical (dm(nn, 1));
     dm(nnzd, 2) = abs (dm(nnzd, 2));
   endif

if this does the job it should probably have some self tests added to the end to verify.

(file #53220)

Nicholas Jankowski <nrjank>
Project Member
Mon 18 Apr 2022 03:52:38 PM UTC, comment #1: 

Thanks.
The patch isn't a patch sensu stricto but rather a complete function w/o a trace what has been changed.

@B.Higgins:
Can you please provide a patch along the lines of:
patch -u <orig_degrees2dm> <modified_degrees2dm> > <patchfile>
?
A Mercurial changeset would even be better but I gather you're not able to make that.

Or otherwise can you tell me which mapping package version you were using?

Philip Nienhuis <philipnienhuis>
Project Member
Mon 18 Apr 2022 11:53:18 AM UTC, original submission:  

I have received the following email from Brendon Higgins (added to CC) with a bug report to degrees2dm and patch (attached):

> I  have found that the OctaveForge function degrees2dm(deg) fails when any
> value in the input vector deg is NaN:
> >> degrees2dm([NaN 3 6757 23421 NaN 1.2345])
> error: invalid conversion from NaN to logical
> error: called from
>     degrees2dm at line 90 column 10
>
> In comparison, Matlab's implementation handles this input gracefully by
> returning [NaN, NaN] for each such value.
>
> For your consideration, I attach a modified version of dergees2dm.m, wherein
> I've attempted to avoid the call to logical() for dm rows containing NaN. It
> would be great if a future version contained such a fix.

Carnë Draug <carandraug>
Project Member

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #53220:  mapping_degrees2dmNaNfix_bug62330.diff added by nrjank (763B - application/octet-stream)
file #53104:  degrees2dm.m added by carandraug (4KiB - text/x-objcsrc - fixed implementation submitted by Brendin Higgins by email)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by nrjank (Updated the item)
  • -email is unavailable- added by philipnienhuis
  • -email is unavailable- added by carandraug (Submitted the item)
  • -email is unavailable- added by carandraug
  •  

    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 project members can vote.

     

     

     

     

    Follow 10 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2022-07-15 philipnienhuis StatusPatch Submitted Fixed
        Assigned toNone philipnienhuis
        Open/ClosedOpen Closed
    2022-05-17 nrjank StatusNeed Info Patch Submitted
        Originator Name Brendon Higgins
    2022-05-17 nrjank Attached File- Added mapping_degrees2dmNaNfix_bug62330.diff, #53220
    2022-04-18 philipnienhuis Carbon-Copy- Added philipnienhuis
    2022-04-18 philipnienhuis StatusPatch Submitted Need Info
    2022-04-18 carandraug Attached File- Added degrees2dm.m, #53104
        Carbon-Copy- Added -email is unavailable-

    Back to the top


    Powered by Savane 3.9