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

 
 

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

Submitted by:  Arpit Sharma <arpit17792>
Submitted on:  Wed 05 Mar 2014 05:02:45 AM UTC  
 
Category:  Octave Forge 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

Add a New Comment (Rich Markup)
   

You are not logged in

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

 

( 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>
Project 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>
Project 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>
Project 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
  •  

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

    Only project 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 Forge Package
    2014-03-05 arpit17792 Carbon-Copy- => Added arpit17792@gmail.com

    Back to the top


    Powered by Savane 3.5