bugGNU Octave - Bugs: bug #41776, Function unitsratio giving...

 
 

bug #41776: Function unitsratio giving incorrect result for unitsratio("degree","radian")

Submitter:  Arpit Sharma <arpit17792>
Submitted:  Wed 05 Mar 2014 05:02:45 AM UTC
   
 
Category:  Octave Package Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Incorrect Result
Status:  Fixed Assigned to:  None
Originator Name:  Arpit Sharma Open/Closed:  * Closed
Release:  * other Operating System:  * Any
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Fri 07 Mar 2014 03:39:27 AM UTC, comment #7: 

@Felipe

Yes. Thinking in "from->to" makes a lot more sense to me and the implementation follows that. Most of the functions for conversions are named like this, e.g., rad2deg, rgb2ind, sub2ind, etc.

On the usage, we don't have a choice, we must mimic Matlab.

Carnë Draug <carandraug>
Group Member
Thu 06 Mar 2014 08:15:57 PM UTC, comment #6: 

so the usage is:

    unitsratio (to, from)

but the implementation is:

    ratios.(valid_from).(valid_to)

?

Felipe G. Nievinski <fgnievinski>
Thu 06 Mar 2014 08:11:53 PM UTC, comment #5: 

@Filipe

the usage of unitsratio is:


unitsratio (to, from)


1 radian is 180/pi degrees, therefore


assert (unitsratio ("degrees", "radians"), 180 / pi)


is correct.

Carnë Draug <carandraug>
Group Member
Thu 06 Mar 2014 07:54:13 PM UTC, comment #4: 

@Arpit: please note that %!%assert indicates a disabled test -- %!assert is enabled.

@Carne:

the true values seem to be switched between first and second assert:
%!assert (unitsratio ("degrees", "radians"), 180 / pi)
%!assert (unitsratio ("radians", "degrees"), pi / 180)

thanks

Felipe G. Nievinski <fgnievinski>
Thu 06 Mar 2014 07:11:58 PM UTC, comment #3: 

The change you made to the ratio between radians and degrees was wrong. There was a incorrect variable which only had effect for angles on your patch and I pushed that part only.

Also, you mentioned several failing tests. Importing your changeset made reference to another cset that does not exist. This would suggest to me that the reason you had tests failing was due to local changes.

See cset 1f2ceac95993 https://sourceforge.net/p/octave/mapping/ci/1f2ceac959938f086103807f4b0b7bb321ac1dfa for your fix.

Carnë Draug <carandraug>
Group Member
Thu 06 Mar 2014 05:14:51 AM UTC, comment #2: 

Okay submitted patch with modifications.

thanks

(file #30786)

Arpit Sharma <arpit17792>
Wed 05 Mar 2014 07:27:21 AM UTC, comment #1: 

there's a typo in line 59:

    valid_to    = validatestring (from, {"radians", "degrees"});

compare it to line 60:

    valid_from  = validatestring (from, {"radians", "degrees"});

do you see it?


can you please work on a patch?

also please enable and correct the following tests:

%!assert (unitsratio ("degrees", "radians"), 180 / pi)
%!assert (unitsratio ("radians", "degrees"), pi / 180)

the true values seem to be switched between first and second assert.

thanks.

Felipe G. Nievinski <fgnievinski>
Wed 05 Mar 2014 05:02:45 AM UTC, original submission:  

In https://sourceforge.net/p/octave/mapping/ci/default/tree/ function unitsratio is giving incorrect result for unitsratio("degree","radian").


>>> unitsratio ("radian", "degree")
ans =  1


Arpit Sharma <arpit17792>

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #30788:  diff.patch added by arpit17792 (2KiB - text/x-patch - This is updated unitsratio.m please ignore file #30786)
file #30789:  unitsratio.m added by arpit17792 (17KiB - text/x-objcsrc - This is updated unitsratio.m please ignore file #30786)
file #30786:  unitsratio.m added by arpit17792 (17KiB - text/x-objcsrc - Updated unitsratio.m)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by fgnievinski (Posted a comment)
  • -email is unavailable- added by jwe (Updated the item)
  • -email is unavailable- added by arpit17792 (Submitted the item)
  • -email is unavailable- added by arpit17792
  •  

    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 8 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2014-03-06 carandraug Open/ClosedOpen Closed
        Operating SystemGNU/Linux Any
    2014-03-06 carandraug StatusNone Fixed
    2014-03-06 arpit17792 Attached File- Added diff.patch, #30788
        Attached File- Added unitsratio.m, #30789
    2014-03-06 arpit17792 Attached File- Added unitsratio.m, #30786
    2014-03-05 jwe CategoryOctave Function Octave Package
    2014-03-05 arpit17792 Carbon-Copy- Added -email is unavailable-

    Back to the top

    Powered by Savane 3.13-3230.
    Corresponding source code