bugGNU Octave - Bugs: bug #40668, -NA is inconsistent with other...

 
 

bug #40668: -NA is inconsistent with other operations on NA

Submitted by:  Daniel Kraft <domob>
Submitted on:  Fri Nov 22 07:39:27 2013  
 
Category: InterpreterSeverity: 3 - Normal
Priority: 5 - NormalItem Group: Incorrect Result
Status: ConfirmedAssigned to: None
Originator Name: Open/Closed: Open
Release: devOperating System: GNU/Linux

Add a New Comment (Rich MarkupRich Markup):
   

You are not logged in

Please log in, so followups can be emailed to you.

 

(Jump to the original submission Jump to the original submission)

Wed Nov 16 23:17:02 2016, comment #8:

I assume you are asking about unpatched Octave as we know it? That's what I tested here.

I know almost nothing about R, but here is what I tried:

So that's one direction. In the other direction:

Is that what you are looking for? If not what can I test differently?

Mike Miller <mtmiller>
Project Administrator
Mon Nov 25 03:30:12 2013, comment #7:

Could someone try the following and see if it works?

Write NA and -NA to a file in binary format in R and read the file in Octave. Do you get NA for both?

Write NA and -NA to a file in binary format in Octave and read the file in R. Do you get NA for both?

Being able do do this (at least for NA, not sure about -NA) was the original motivation for having NA in Octave implemented like the NA from R. So if it is not possible to do this anymore, we also need to find a way to restore this capability.

John W. Eaton <jwe>
Project Administrator
Mon Nov 25 02:34:21 2013, comment #6:

> Are there possible problems that can be introduced by modifying __lo_ieee_is_NA() accordingly?


Well, the point was to do whatever R was doing. What does R do internally with -NA? In its REPL, -NA is still NA.

Jordi GutiƩrrez Hermoso <jordigh>
Project Administrator
Fri Nov 22 15:36:47 2013, comment #5:

As I understand it, doesn't IEEE 754 allow for distinct (quiet) NaN values, of which NA is a particular one? Those values should be propagated through all computations, so as long as you don't perform operations between NA and NaN but only between NA and ordinary numbers, NA should be preserved (also for external codes that "just" do calculations with the numbers passed in).

The sign bit is a bit different from the NaN payload bits, and thus seemingly the unary minus flips the sign bit even though the payload itself stays the same as it should. Based on this interpretation, it makes sense to me to ignore the sign bit when checking for NA. However, I understand that it is probably best not to rely on this behaviour in one's code (and I have changed mine already accordingly).

Are there possible problems that can be introduced by modifying __lo_ieee_is_NA() accordingly?

Daniel Kraft <domob>
Fri Nov 22 14:09:27 2013, comment #4:

The particular values for NA were taken from R and were chosen so that you could exchange binary data files between Octave and R and preserve NA.

But in more than a decade, nothing else about NA has really been implemented in Octave. And I'm not sure we should start now. It's pretty much impossible for us to guarantee that all operations on NA return NA rather than NaN. It's certainly impossible for us to guarantee that for operations that involve external numerical libraries like ATLAS or OpenBLAS.

John W. Eaton <jwe>
Project Administrator
Fri Nov 22 13:16:17 2013, comment #3:

I wondered about the performance hit of doing it higher up as well. Does your quick hack at least fix the problem?

I also don't know how the specific NA value was chosen and what the implications might be for considering it equal ignoring the sign bit. Was it selected because it's a value that floating point math operations will never evaluate to? Is the same true for the negative value?

Mike Miller <mtmiller>
Project Administrator
Fri Nov 22 08:44:06 2013, comment #2:

The problem here seemingly is that the unary minus changes the sign bit, which makes Octave no longer recognise its special NaN-pattern for NA. A possible fix is to change __lo_ieee_is_NA to check for both patterns (the original one and the one with inverted sign bit), like this:

int
__lo_ieee_is_NA (double x)
{
#if defined (HAVE_ISNAN)
lo_ieee_double t;
t.value = x;
return (isnan (x) && (t.word[lo_ieee_hw] == LO_IEEE_NA_HW || t.word[lo_ieee_hw] == 0xFFF840F4)
&& t.word[lo_ieee_lw] == LO_IEEE_NA_LW) ? 1 : 0;
#else
return 0;
#endif
}

(This is of course not a properly done patch, just a quick hack to try it out.)

Since I'm not an expert with IEEE floating-point numbers and things like that: Is this the correct way to fix this bug, or should we rather add a check to unary minus operations so that they don't invert the sign bit in case of NA? This, however, seems to me as if it would hurt performance for each and every unary minus operation, and would also possibly have to be done in all the different implementations of unary minus (for scalars, complex values, matrices, sparse matrices and so on). Thus to me the change of isna() seems better.

If this sounds like a good approach, I will write a nicer patch (including some comments) and provide a hg commit diff.

Daniel Kraft <domob>
Fri Nov 22 07:40:35 2013, comment #1:

I will try to work on a patch.

Daniel Kraft <domob>
Fri Nov 22 07:39:27 2013, original submission:

Currently, -NA evaluates to NaN, while a lot of other operations on NA result in NA. In particular:

+NA, 0 - NA, -1 * NA, Inf * NA, 0 * NA, NA / 0, 0 / NA, and others (especially of course less edge-case like operations, too).

See also https://mailman.cae.wisc.edu/pipermail/help-octave/2013-November/061056.html. I propose that -NA should evaluate to NA to make the behaviour more consistent.

Daniel Kraft <domob>

 

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

Attach File(s):
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by jordigh (Posted a comment)
  • -unavailable- added by jwe (Posted a comment)
  • -unavailable- added by mtmiller (Updated the item)
  • -unavailable- added by domob (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only project members can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 3 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Wed Nov 16 23:17:43 2016mtmillerRelease3.6.4=>dev
    Fri Nov 22 13:10:43 2013mtmillerItem GroupOther=>Incorrect Result
      StatusNone=>Confirmed

    Back to the top


    Powered by Savane 3.1-cleanup