patchGNU Octave - Patches: patch #8827, mapping package: new functions...

 
 

patch #8827: mapping package: new functions wrapTo 360, 2Pi, 180, Pi

Submitted by:  None
Submitted on:  Tue 08 Dec 2015 08:08:16 PM UTC  
 
Category:  None Priority:  1 - Later
Status:  Done Privacy:  Public
Assigned to:  Philip Nienhuis <philipnienhuis> Originator Email:  -email is unavailable-
Open/Closed:  Closed

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)

Mon 18 Jul 2016 03:14:39 PM UTC, comment #24: 

No comments => closing report

Philip Nienhuis <philipnienhuis>
Project MemberIn charge of this item.
Sun 07 Feb 2016 07:51:25 PM UTC, comment #23: 

Now that anew mapping package release is out with these new functions I've set status to "postponed" as mentioned in comment #21. If no further improvements or modifications turn up here in say 3 months, I'll close this patch report.

Once again thank you for these contributions and your patience.

Philip Nienhuis <philipnienhuis>
Project MemberIn charge of this item.
Sat 02 Jan 2016 12:11:33 AM UTC, comment #22: 
Philip Nienhuis <philipnienhuis>
Project MemberIn charge of this item.
Tue 29 Dec 2015 02:09:54 PM UTC, comment #21: 

??? irritating markup.

As to 3:
I had no idea of performance of either set of versions so I tried:

## Get 10^8 reals in [-1 .. 1]
>> testarr = 2 * (rand (10000) - 0.5);
## transform to [-3*pi .. 3*pi]
>> c = 3 * pi * testarr;
>> mean (c(:))
ans =  -6.9368e-004
>> max (c(:))
ans =  9.4248
>> min (c(:))
ans = -9.4248
>> numel (find (c < 0))
ans =  50006525
>> numel (find (c > 0))
ans =  49993475
>> 2 * mean (c(find (c > 0)))
ans =  9.4250
>> 2 * mean (c(find (c < 0)))
ans = -9.4251

... so the results appear to be evenly distributed in [-3*pi .. 3*pi].

Then, with my wrapTo2Pi.m version:

>> tic; wrapTo2Pi (c); toc
Elapsed time is 6.84839 seconds.
>> tic; wrapTo2Pi (c); toc
Elapsed time is 6.9144 seconds.
>> tic; wrapTo2Pi (c); toc
Elapsed time is 6.86239 seconds.

and with yours:

>> tic; wrapTo2Pi (c); toc
Elapsed time is 9.71456 seconds.
>> tic; wrapTo2Pi (c); toc
Elapsed time is 9.68055 seconds.
>> tic; wrapTo2Pi (c); toc
Elapsed time is 9.68155 seconds.

and as your wrapTo2Pi is called by your other wrapTo... functions I suppose those must be slower still due to function call overhead.

Early next year I'll push my versions to the repo, maybe after a little more polishing.
FYI I'm still busy with GIS raster file I/O + speeding up .shp file drawing (esp. filled polygons), one that works sufficiently reliably I'll make a new release. Once there I'll set status to "postponed".

Philip Nienhuis <philipnienhuis>
Project MemberIn charge of this item.
Tue 29 Dec 2015 02:07:43 PM UTC, comment #20: 

Hmm, post cut off. Try #2

As to 3:
I had no idea of performance of either set of versions so I tried:

Philip Nienhuis <philipnienhuis>
Project MemberIn charge of this item.
Tue 29 Dec 2015 02:05:00 PM UTC, comment #19: 

@Oscar:
(a somewhat long post, but I am very grateful for your continued interest and perseverance)

My wrapTo180 is correct, but the devil is in the details. From the test suite:

>> c = [-721.1, -718.9, -481.3, -479.99, -361, -359, -200, -180-(1e-14), -180, ...
     -180-(2e-14), -160, -eps, 0, eps, 160, 180, 180+(1e-14), 180+(2e-14), 200];
c =
Columns 1 through 6:
  -721.10000  -718.90000  -481.30000  -479.99000  -361.00000  -359.00000
Columns 7 through 12:
  -200.00000  -180.00000  -180.00000  -180.00000  -160.00000    -0.00000
Columns 13 through 18:
     0.00000     0.00000   160.00000   180.00000   180.00000   180.00000
Column 19:
   200.00000
>> [c; wrapTo180(c)]
ans =
Columns 1 through 6:
  -721.10000  -718.90000  -481.30000  -479.99000  -361.00000  -359.00000
    -1.10000     1.10000  -121.30000  -119.99000    -1.00000     1.00000
Columns 7 through 12:
  -200.00000  -180.00000  -180.00000  -180.00000  -160.00000    -0.00000
   160.00000  -180.00000  -180.00000   180.00000  -160.00000    -0.00000
Columns 13 through 18:
     0.00000     0.00000   160.00000   180.00000   180.00000   180.00000
     0.00000     0.00000   160.00000   180.00000   180.00000  -180.00000
Column 19:
   200.00000
  -160.00000

Entry 18 looks wrong, but as you can see in the definition of "c", it is deliberately set to a value just a tiny little bit over the numerical accuracy treshold > 180. You don't see that due to rounding in the output.
FYI, the treshold (numerical accuracy) is outlined in comment #13, for values near 180 and -180 it amounts to ~1e-14. The 18th entry of "c" is 180+2e-14 so is larger => should wrap around.
Similarly for entry #10 (-180-2e-14).
Your version apparently doesn't pick up that tiny difference.

I think I'll use my versions, my motives are as follows (comments welcome nevertheless), in decreasing order of importance:

1. Code should be correct - in this case "Matlab-correct" and I do think there's a fair case for Matlab's peculiar interval end point behavior;

2. Code should be maintainable;

3. Code should be fast.

(yeah the order of 2. and 3. leave room for discussion :-) )

As to 1:
After a fair bit of experimenting and thinking over I do not see how to reliably obtain Matlab's end point behavior in a symmetrical way without a statement containing "abs(x) > {positive_endpoint}" plus some call to sign(), so that the entire interval incl. endpoints is not affected (wrapped).

As to 2:
I find my own code a tiny little easier to follow than your code :-) but it is entirely possible that my perception will wear out over time. I therefore leave your contributions here with a "postponed" until some later mapping package release so that it can be further polished.

As to 3:
I had no idea of performance of either set of versions so I tried:

Philip Nienhuis <philipnienhuis>
Project MemberIn charge of this item.
Mon 28 Dec 2015 09:09:42 PM UTC, comment #18: 

Hello Philip,

thank you very much for your functions. I had a look on you code and I've tested the performance of both algorithms. My approach seems to by faster. I've updated my code with your template including the tests.

In order to increase the performance of all the functions I've implemented the algorithm in all of them instead of calling wrapTo2Pi with angle scaling on the others. This approach works better.

Regarding the tests, I've located an error in the 18th component in you wrapTo180.m. ¿Could you confirm it?, it is stated that the output or wrapTo180(180) = -180 and it should be 180. Am I correct?

Feel free to use your algorithm or mine, both work correctly.

Regards,

Òscar

(file #35879)

Anonymous
Mon 28 Dec 2015 07:21:28 PM UTC, comment #17: 

Oops, I now see that I didn't manage to upload my "final" versions.

@Oscar, please have a look at them and compare with your versions.

(file #35878)

Philip Nienhuis <philipnienhuis>
Project MemberIn charge of this item.
Mon 28 Dec 2015 04:06:17 PM UTC, comment #16: 

Hello,

It's me again. Sorry for the buggy versions. The initial one worked for me without taking into account the issue of the edges. I've prepared a new set of functions which, compared with Matlab results, work fine.

Find them attached in the wrap2v2.7z. I hope you like them, they work fine in Octave4.0 but, if executed on Matlab, they do not work well. I think it has something to do with the second line in wrapTo2Pi.m function.

wrapToPi, wrapTo180 and wrapTo360 just call wrapTo2Pi using the proper scaling factor.

In the z7 file I include some test vectors in which the "v" variable is the input to the wrapToxxx function and the "m" vector is the expected result.

Best regards,

Òscar

(file #35875)

Anonymous
Fri 25 Dec 2015 09:43:45 PM UTC, comment #15: 

Gentlemen,

I've combined Oscar's, Andy's and my own mods to arrive at the versions in attached archive.
When I'm back from holiday (after New Years Eve) I'll push them and probably make a new mapping package release.
If in the mean time you see opportunities for tuning these files, please go ahead.

Thanks very much for your cooperation.

Philip Nienhuis <philipnienhuis>
Project MemberIn charge of this item.
Thu 24 Dec 2015 07:50:01 PM UTC, comment #14: 

@Andy
Only now I see your versions, thanks.
I'll look at the whole lot later this week(end) - back to XMas-dinner...

Philip Nienhuis <philipnienhuis>
Project MemberIn charge of this item.
Thu 24 Dec 2015 07:47:23 PM UTC, comment #13: 

Here's my version of wrapTo180.m, including a test.

I find that due to numerical inaccuracy, the interval actually is [-180-(e-14)  180+(e-14)] but let's call that reasonable :-)

@Oscar:
In your defense I admit that this isn't as easy as it looks at first sight. Esp. the closed interval at both ends is challenging, otherwise it would have been a simple
rem(X+shift>, <interval_length>) - <shift>

Maybe you can use my wrapTo180.m version as a template.

(file #35853)

Philip Nienhuis <philipnienhuis>
Project MemberIn charge of this item.
Thu 24 Dec 2015 06:21:11 PM UTC, comment #12: 

This has been mentioned before on comment #2.

Can you please add some tests for this functions? The multiple cases by Philip Nienhuis would work nicely.  For example, at the end of wrapToPi, you can add the tests:

%!assert (wrapToPi (pi * [-3:.5:3]),  pi * [-1 -.5 0 -.5 -1 -.5  0 .5 1 .5 0 .5 1], eps)
%!assert (wrapToPi ([Inf NaN pi]),  [XX XX pi], eps)
## Test more than one dimensional vector
%!assert (wrapToPi (pi .* [-2 -1/2 0; 2 1/3 5/3]),  pi* [0 -1/2 0; 1 1/3 2/3], eps)

Carnë Draug <carandraug>
Project Member
Thu 24 Dec 2015 06:15:47 PM UTC, comment #11: 

Attached a fixed version of the four files with tests and fixed texinfo.

It doesn't follow the strange Matlab boundary convention, for example,  to map -180 to -180 and 180 to 180.

I've removed the author, created timestamp and version line because this should be handled by our version control system (mercurial).

(file #35851)

Andreas Weber <andy1978>
Project Member
Thu 24 Dec 2015 03:45:20 PM UTC, comment #10: 

Unfortunately, wrapTo180 has issues as well:

>> wrapTo180 (-181)
ans = -181

Philip Nienhuis <philipnienhuis>
Project MemberIn charge of this item.
Tue 22 Dec 2015 06:59:56 PM UTC, comment #9: 

@comment #7

Yeah I overlooked the second minus sigh (oops), apologies.

But then I tried your latest wrapToPi (of comment #8 file #35824) to get:

>> a = wrapToPi (pi * [-10 : 1e-1 : 10]);
>> a (find (abs (a) > pi))
ans =
Columns 1 through 8:
  -5.9690  -5.6549  -5.3407  -5.0265  -4.7124  -4.3982  -4.0841  -3.7699
Columns 9 through 16:
  -3.4558  -5.9690  -5.6549  -5.3407  -5.0265  -4.7124  -4.3982  -4.0841
Columns 17 through 24:
  -3.7699  -3.4558  -5.9690  -5.6549  -5.3407  -5.0265  -4.7124  -4.3982
Columns 25 through 32:
  -4.0841  -3.7699  -3.4558  -5.9690  -5.6549  -5.3407  -5.0265  -4.7124
Columns 33 through 40:
  -4.3982  -4.0841  -3.7699  -3.4558  -5.9690  -5.6549  -5.3407  -5.0265
Columns 41 through 45:
  -4.7124  -4.3982  -4.0841  -3.7699  -3.4558

... where I see lots of wrapToPi outcomes outside of the [-pi, +pi] interval... 
In fact this would be a nice base for a test.

Along those lines,

>> b = wrapTo180 (180 * [-5: 1 : 5]);
>> b (find (abs (b) > 180))
ans = [](1x0)

(...but we already agreed this seems to work fine),

>> c = wrapTo2Pi (2*pi * [-3 : 1e-1 : 3]);
>> c (find (c < 0 | c > 2*pi)) ## Note: single "|" !
ans =
Columns 1 through 7:
   -6.28319   -5.65487   -5.02655   -4.39823   -3.76991   -3.14159   -2.51327
Columns 8 through 14:
   -1.88496   -1.25664   -0.62832  -11.93805  -11.30973  -10.68142  -10.05310
Columns 15 through 21:
   -9.42478   -8.79646   -8.16814   -7.53982   -6.91150   -6.28319   -5.65487
Columns 22 through 28:
   -5.02655   -4.39823   -3.76991   -3.14159   -2.51327   -1.88496   -1.25664
Column 29:
   -0.62832

,,which again doesn't match what Matlab says: "... wraps angles in lambda, in radians, to the interval [0 2*pi]" and I see negative values in the outcome,

>> b = wrapTo360 (360 * [-5: 1 : 5]);
error: 'sgn' undefined near line 36 column 11
error: called from
    wrapTo360 at line 36 column 9

(....Hey!...)
In you latest wrapTo360, I clearly see:
:
  xwrap = sgn.*(x - 360*(x > 360));
:

Please, in your Octave terminal type:

which sgn

and report back.

Philip Nienhuis <philipnienhuis>
Project MemberIn charge of this item.
Tue 22 Dec 2015 01:56:18 PM UTC, comment #8: 

For the sgn error, the last functions do not use the sgn function!

Correct files:
file #35821:  wrapTo2Pi.m afegit per None (1kB - application/octet-stream - FINAL versions, correct sign issue and bug in the previous version)
file #35822:  wrapTo360.m afegit per None (1kB - application/octet-stream - FINAL versions, correct sign issue and bug in the previous version)
file #35823:  wrapTo180.m afegit per None (1kB - application/octet-stream - FINAL versions, correct sign issue and bug in the previous version)
file #35824:  wrapToPi.m afegit per None (1kB - application/octet-stream - FINAL versions, correct sign issue and bug in the previous version)

please, try the last version.

Anonymous
Tue 22 Dec 2015 01:54:17 PM UTC, comment #7: 

I thing that the results are correct:

Matlab helps states: (In general, odd, positive multiples of pi map to pi and odd, negative multiples of pi map to −pi.)

which means that pi, 3pi, (2*n+1)pi (n=1,2....) map to pi whereas -pi, -3pi... map to -pi which is what is happening.

I believe your interpretation of the Matlab help is not correct.

Regards,

Òscar

Anonymous
Mon 21 Dec 2015 09:54:44 PM UTC, comment #6: 

Thanks for the patches.

I now get:

>> wrapToPi ([-pi, -3*pi])
ans =
  -3.1416  -3.1416

which is still incorrect, expected answer = [ -3.1416  3.1416 ]
wrapTo2Pi.m has the same issue.

>> wrapTo360 ([0, 180, 360, -1, 361])
error: 'sgn' undefined near line 36 column 11
error: called from
    wrapTo360 at line 36 column 9

Did you use the "sgn" function from Matlab File Exchange?
http://nl.mathworks.com/matlabcentral/fileexchange/23571-short-time-energy-and-zero-crossing-rate/content/stezcr/sgn.m
=> we cannot use functions from there as the terms of use forbid usage with any other program than Matlab.

>> wrapTo180 ([0, 180, -180, -1, 181])
ans =
     0   180  -180    -1  -179

That one looks good.

Philip Nienhuis <philipnienhuis>
Project MemberIn charge of this item.
Mon 21 Dec 2015 03:14:27 PM UTC, comment #5: 

Wow... I do not really understand this forum post process...

(file #35821, file #35822, file #35823, file #35824) work fine

I was referring to files which had a huge bug

(file #35816, file #35817, file #35818, file #35819)

I solved the sign problem using "rem" function instead of "mod".

Regards,

Òscar

Anonymous
Mon 21 Dec 2015 03:12:22 PM UTC, comment #4: 

Forget the updated files... they do not work. I'll try to fix the error today.

(file #35821, file #35822, file #35823, file #35824)

Anonymous
Mon 21 Dec 2015 02:19:05 PM UTC, comment #3: 

Hello,

I'm sorry about the sign issue, I have no access to a matlab with the appropriate toolbox.

I've updated the four m files so they have into account the sign issue and solve it.

I've also change the end by endfunction as stated in your comment.

(file #35816, file #35817, file #35818, file #35819)

Anonymous
Fri 11 Dec 2015 12:18:37 PM UTC, comment #2: 

(Also see discussion here:
http://octave.1599824.n4.nabble.com/wrapToPi-function-tt4673887.html)

@Oscar
Please supply some tests for these functions.
Also use "endfunction" rather than "end" and indent the actual code w. 2 spaces. Also consider what I wrote in Comments (below).

Once done, I'll do some additional but minor style edits before adding them to the mapping package. I plan to make a new mapping package release quite soon (next week), hopefully you'll be in time to have these functions included.

Some comments:
==============
I'm not in favor of camel-casing function names. I'd prefer wraptopi or even wrap2pi rather than wrapToPi) but it seems Matlab introduced this sillyness here so we'd need to follow for the sake of Matlab compatibility :-)

I tried "wrapToPi ([-pi, -3*pi])" and got [3.1416 3.1416] as answer. AFAIU that's not in line with what is stated by TMW here:
http://nl.mathworks.com/help/map/ref/wraptopi.html
where they say that "odd, negative multiples of pi map to −pi" => I'd expect [-3.1416 3.1416]

Similarly,

>> wrapTo360 ([0, 180, 360, -1, 361])
ans =
     0   180   360   259     1
## Shouldn't that be [0 180 360 359 1] ?

>> wrapTo180 ([0, 180, -180, -1, 181])
ans =
     0   180   180    -1  -179
## Expected [0 180 -180 -1 -179]

...which AFAIU indicates that there's some polishing needed to get Matlab-compatible answers.
(Mathematically I think your functions are mostly correct but Matlab apparently does something odd regarding closed/open intervals at the lowest interval extremes. And we'd have to follow that....)

Philip Nienhuis <philipnienhuis>
Project MemberIn charge of this item.
Thu 10 Dec 2015 06:02:30 PM UTC, comment #1: 

Thanks, I'll have a look at it.

(mapping pkg maintainer)

Philip Nienhuis <philipnienhuis>
Project MemberIn charge of this item.
Tue 08 Dec 2015 08:08:16 PM UTC, original submission:  

This functions implements the wrapToPi and wrapTo180 function available in Matlab as m files. 

If accepted, they should be added to  Octave "mapping" package.

Anonymous

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #35879:  WrapTo.tar.gz added by None (2KiB - application/gzip - Oscar updated algorithm with Philip template)
file #35878:  wrap_PRN.tar.gz added by philipnienhuis (2KiB - application/x-gzip)
file #35875:  wrapTo.7z added by None (21KiB - application/x-7z-compressed - New versions with sample arrays from Matlab function)
file #35853:  wrapTo180.m added by philipnienhuis (2KiB - text/x-objective-c)
file #35851:  wrapTo.tar.gz added by andy1978 (1KiB - application/gzip)
file #35840:  wrapTo360.m added by None (1KiB - application/octet-stream - Updated version without)
file #35821:  wrapTo2Pi.m added by None (1KiB - application/octet-stream - FINAL versions, correct sign issue and bug in the previous version)
file #35822:  wrapTo360.m added by None (1KiB - application/octet-stream - FINAL versions, correct sign issue and bug in the previous version)
file #35823:  wrapTo180.m added by None (1KiB - application/octet-stream - FINAL versions, correct sign issue and bug in the previous version)
file #35824:  wrapToPi.m added by None (1KiB - application/octet-stream - FINAL versions, correct sign issue and bug in the previous version)
file #35816:  wrapTo2Pi.m added by None (1KiB - application/octet-stream - with two space identation)
file #35817:  wrapTo360.m added by None (1KiB - application/octet-stream - with two space identation)
file #35818:  wrapTo180.m added by None (1KiB - application/octet-stream - with two space identation)
file #35819:  wrapToPi.m added by None (1KiB - application/octet-stream - with two space identation)
file #35812:  wrapToPi.m added by None (1KiB - application/octet-stream - Updated versions taking into account the sign issue on the edges)
file #35813:  wrapTo360.m added by None (1KiB - application/octet-stream - Updated versions taking into account the sign issue on the edges)
file #35814:  wrapTo180.m added by None (1KiB - application/octet-stream - Updated versions taking into account the sign issue on the edges)
file #35815:  wrapTo2Pi.m added by None (1KiB - application/octet-stream - Updated versions taking into account the sign issue on the edges)
file #35688:  wrapTo360.m added by None (1KiB - application/x-download - Two more functions, which complete my current contribution to this package)
file #35689:  wrapTo2Pi.m added by None (1KiB - application/x-download - Two more functions, which complete my current contribution to this package)
file #35686:  wrapTo180.cc added by None (3KiB - text/x-c++src - Also its .cc versions)
file #35687:  wrapToPi.cc added by None (3KiB - text/x-c++src - Also its .cc versions)
file #35669:  wrapToPi.m added by None (1KiB - application/x-download)
file #35670:  wrapTo180.m added by None (1KiB - application/x-download)

 

Depends on the following items: None found

Items that depend on this one: None found

 

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

    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 logged-in users can vote.

     

     

     

    Follow 25 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2016-07-18 philipnienhuis StatusPostponed => Done
        Open/ClosedOpen => Closed
    2016-02-07 philipnienhuis Priority3 - Low => 1 - Later
        StatusDone => Postponed
    2016-01-02 philipnienhuis Priority5 - Normal => 3 - Low
        StatusReady For Test => Done
    2015-12-28 None Attached File- => Added WrapTo.tar.gz, #35879
    2015-12-28 philipnienhuis Attached File- => Added wrap_PRN.tar.gz, #35878
    2015-12-28 None Attached File- => Added wrapTo.7z, #35875
    2015-12-25 philipnienhuis StatusNone => Ready For Test
        Assigned toNone => philipnienhuis
    2015-12-24 philipnienhuis Attached File- => Added wrapTo180.m, #35853
    2015-12-24 andy1978 Attached File- => Added wrapTo.tar.gz, #35851
    2015-12-22 None Attached File- => Added wrapTo360.m, #35840
    2015-12-21 None Attached File- => Added wrapTo2Pi.m, #35821
        Attached File- => Added wrapTo360.m, #35822
        Attached File- => Added wrapTo180.m, #35823
        Attached File- => Added wrapToPi.m, #35824
    2015-12-21 None Attached File- => Added wrapTo2Pi.m, #35816
        Attached File- => Added wrapTo360.m, #35817
        Attached File- => Added wrapTo180.m, #35818
        Attached File- => Added wrapToPi.m, #35819
    2015-12-21 None Attached File- => Added wrapToPi.m, #35812
        Attached File- => Added wrapTo360.m, #35813
        Attached File- => Added wrapTo180.m, #35814

    Back to the top


    Powered by Savane 3.5