bugGNU Octave - Bugs: bug #52342, vecnorm command

 
 

bug #52342: vecnorm command

Submitted by:  CH <atcl>
Submitted on:  Sun 05 Nov 2017 09:54:30 PM UTC  
 
Category: InterpreterSeverity: 3 - Normal
Priority: 5 - NormalItem Group: Matlab Compatibility
Status: FixedAssigned to: None
Originator Name: Open/Closed: Closed
Release: devOperating 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 Jump to the original submission)

Thu 09 Nov 2017 06:00:28 AM UTC, comment #10:

I checked in a patch here http://hg.savannah.gnu.org/hgweb/octave/rev/3d96400df713.

I added validation for the dimension argument, and added more complete code to detect the first non-singleton dimension. I also added a separate TeX-formatted definition of the p-norm to the docstring so that it appears nicely in the Octave Manual.

In terms of performance, I found for the case of n == 2 that using sumsq() resulted in a 15X speed-up over abs(). This is much faster than using the vectorized real^2 + imag^2 as well. Finally, the vectorized code seems to outperform abs() when p is evenly divisble by 2. So I split the generic calculation based on whether p is even or odd and use vectorized cod or abs() appropriately.

Marking as fixed and closing report.

Rik <rik5>
Project Administrator
Wed 08 Nov 2017 07:20:47 AM UTC, comment #9:

Here it is my third patch.

(file #42346)

Marco Caliari <caliari>
Project Member
Tue 07 Nov 2017 07:36:26 AM UTC, comment #8:

@Rik: thanks, but even with your benchmark I get

The vectorized code is in general faster, even for the most common p=2 (and apart from the weird cases). This is true both in 4.2.1 and 4.3.0+. It was always true for me: in fact, I teach my students this trick since a long time.
Anyway, this is a different topic, I can write vecnorm in the easy way (with abs).

Marco Caliari <caliari>
Project Member
Tue 07 Nov 2017 06:00:07 AM UTC, comment #7:

@Marco: Benchmarking is pretty hard to get right. First, one generally needs to repeat the test several times. Second, The way your test was structured on one line means that the CPU's dynamic frequency scaling was probably not fully engaged for the first calculation using abs(), but was for the second part of the test. That gives an unfair advantage to the vectorized code.

I wrote a small benchmark script (bm_abs.m) which is attached.

The results for various p values are shown below:

Definitely some weirdness associated with p = [4,6]. I don't think those are common choices, so it may be best to stay with abs().

Rik <rik5>
Project Administrator
Mon 06 Nov 2017 03:06:55 PM UTC, comment #6:

My analysis without any deeper interpreter knowledge would be, that for p=2 => p/2 = 1 and p=4 => p/2 = 2 either nothing, or a more efficient sqrt is used than the general pow function (probably something similar for p=6).

In general, I guess the second variant is faster because it is better vectorized.

So, then the abs should also be replaced in the 1- and Inf norm.

Best

atcl

CH <atcl>
Mon 06 Nov 2017 02:22:48 PM UTC, comment #5:

Sorry, I used the conj function but I had in mind the sum of the squared real and imaginary parts. In fact, I thougth that computing

is faster than

But, I see a strange behavior:

So, (real^2+imag^2)^(p/2) is always faster, except when p=3. In fact, it is elevation to power 2 or 3 to be very fast. Is it just me?

Marco Caliari <caliari>
Project Member
Mon 06 Nov 2017 01:50:37 PM UTC, comment #4:

This looks better.
Though I don't understand why the general p-norm and 2-norm are handled differently than the Inf- and 1-norm in terms of the absolute value. Shouldn't be abs() used everywhere?

Also, an unsuitable "dim" argument needs to handled, I guess.

Thanks

atcl

CH <atcl>
Mon 06 Nov 2017 01:36:44 PM UTC, comment #3:

@CH: thanks for your suggestions. Here it is a new version (p=-Inf is not allowed in Matlab R2017b).

(file #42335)

Marco Caliari <caliari>
Project Member
Mon 06 Nov 2017 11:00:09 AM UTC, comment #2:

I would suggest making it a switch block and add special cases for p={1,2} for performance. Furthermore, Matlab also allows -Inf, and the case of p<=0, complex p or vector/matrix-valued p should be handled, the latter with the appropriate error.

I assume this function was added for performance reasons, ie to not use cellfun, so maybe this should be a builtin at some point later on?

Best

atcl

CH <atcl>
Mon 06 Nov 2017 09:29:09 AM UTC, comment #1:

What about the included patch? I'm not sure I found the best way to discover the first non-singleton dimension (first if branch).

(file #42333)

Marco Caliari <caliari>
Project Member
Sun 05 Nov 2017 09:54:30 PM UTC, original submission:

Matlab recently introduced the

command for vector norm computation (of type t) for a set vectors (column or row depending on d) grouped into A.

CH <atcl>

 

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

Attach File(s):
   
   
Comment:
   

Attached Files
file #42346:  vecnorm.diff3 added by caliari (4KiB - application/octet-stream)
file #42340:  bm_abs.m added by rik5 (160B - text/x-matlab)
file #42335:  vecnorm.diff2 added by caliari (4KiB - application/octet-stream)
file #42333:  vecnorm.diff added by caliari (4KiB - application/x-tex)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by rik5 (Posted a comment)
  • -unavailable- added by caliari (Updated the item)
  • -unavailable- added by atcl (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 7 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Thu 09 Nov 2017 06:00:28 AM UTCrik5StatusPatch Submitted=>Fixed
      Open/ClosedOpen=>Closed
    Wed 08 Nov 2017 07:20:47 AM UTCcaliariAttached File-=>Added vecnorm.diff3, #42346
    Tue 07 Nov 2017 06:00:42 AM UTCrik5Attached File-=>Added bm_abs.m, #42340
    Mon 06 Nov 2017 01:36:44 PM UTCcaliariAttached File-=>Added vecnorm.diff2, #42335
    Mon 06 Nov 2017 09:29:09 AM UTCcaliariAttached File-=>Added vecnorm.diff, #42333
      StatusNone=>Patch Submitted

    Back to the top


    Powered by Savane 3.1-cleanup1