patchGNU Octave - Patches: patch #9065, octave::math::min and max template...

 
 

patch #9065: octave::math::min and max template ignore isnan

Submitted by:  Carnë Draug <carandraug>
Submitted on:  Tue 02 Aug 2016 03:29:36 AM UTC  
 
Category: NonePriority: 5 - Normal
Status: NonePrivacy: Public
Assigned to: NoneOpen/Closed: Open

Add a New Comment(Rich Markup)
   

You are not logged in

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

 

Tue 02 Aug 2016 12:42:18 PM UTC, comment #4:

> Why do you need or care about long double? I omitted those because they were not used in Octave, nor do I expect them to be.


If liboctave is not meant to be used with types other than double, float, and octave_int's then this is not a problem (and this patch only reduces a bit of code duplication).

But aren't we moving in the direction of making this a C++ library more useful by other projects? I think should be considering other types and defining this operations better so it's clear what min and max are meant to compute for types we don't know about, and the requirements on those types.

Carnë Draug <carandraug>
Project Member
Tue 02 Aug 2016 06:51:42 AM UTC, comment #3:

Why do you need or care about long double? I omitted those because they were not used in Octave, nor do I expect them to be.

John W. Eaton <jwe>
Project Administrator
Tue 02 Aug 2016 06:48:01 AM UTC, comment #2:

We have octave_int so that we can provide the saturation semantics for integers that is required for Matlab compatibility.

John W. Eaton <jwe>
Project Administrator
Tue 02 Aug 2016 04:35:04 AM UTC, comment #1:

Draft proposal at https://bitbucket.org/carandraug/octave/commits/e01c4646def1cb06518c6565744e83b601fcb61a

Note that I choose to go with std::max and std::min instead of writing it ourselves for the octave_int specializations (not sure if that could cause performance issues).

Carnë Draug <carandraug>
Project Member
Tue 02 Aug 2016 03:29:36 AM UTC, original submission:

Octaves definition of min is the smallest of the non NaN values, but liboctave's templated octave::math::min function does not behave that way. Instead, it behaves like std::min and we have a specialization for double and float.

This is confusing because: 1) the template behaviour is different from Octave, 2) the specialization actually changes the english definition of Octave's min (given another type, other than double and low, with a concept of nan, min would not consider the nan value). An actual example:

I tried to change this so that the general template does check for nan, while keeping the specialization for octave_int.

This caused problems with int and short because they are missing isnan. These were required because Octave declares MArray<int> and MArray<short> so I defined isnan for int and short (although I'm not sure why, MArray<int> and MArray<short>, are not used anywhere and we already have Marray for all the octave_int --- or why do we have octave_int at all?).

Carnë Draug <carandraug>
Project Member

 

(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 jwe (Posted a comment)
  • -unavailable- added by carandraug (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 logged-in users can vote.

     

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

     

     

    No Changes Have Been Made to This Item

    Back to the top


    Powered by Savane 3.1-cleanup1