patchGNU Octave - Patches: patch #9717, [octave forge] (image) new...

 
 

patch #9717: [octave forge] (image) new function imapplymatrix

Submitter:  Martin Janda <tojtoj>
Submitted:  Fri 09 Nov 2018 01:12:02 AM UTC
   
 
Category:  Forge : new function Priority:  5 - Normal
Status:  Done Privacy:  Public
Assigned to:  None Open/Closed:  Closed
* Mandatory Fields

Add a New Comment Rich Markup
   

Tue 02 Feb 2021 11:36:46 AM UTC, comment #5: 

Closing as done. Thanks Martin & Hartmut.

Avinoam Kalma <avinoam>
Group Member
Mon 01 Feb 2021 08:20:20 PM UTC, comment #4: 

Thanks for the review, Avinoam. And thanks for this new function again, Martin Janda.

I have now pushed the patch (V2) to the image repo: http://hg.code.sf.net/p/octave/image/rev/757fd547b1ba

This issue can be CLOSED now.

Hartmut <hardy>
Mon 01 Feb 2021 07:43:01 PM UTC, comment #3: 

@Hartmut:

Thanks for the patch.
I have checked it and all the tests passed
Please push it.

Avinoam Kalma <avinoam>
Group Member
Sun 31 Jan 2021 10:02:18 PM UTC, comment #2: 

I have now prepared and attached a patch file (V2) that adds this new function under the name of Martin Janda, as well as do the necessary bookkeeping. (It also adds the semicolon after line 181.)

Let me know if you have any objections against pushing this in roughly a week's time.

(file #50816)

Hartmut <hardy>
Mon 25 Jan 2021 09:05:21 PM UTC, comment #1: 

@Martin Janda: Thank you for this nice piece of code for the Octave image package. And I feel sorry that it took so long for you to get any feedback on it.

@Carne, @Avinoam: I would like to push this new function "imapplymatrix.m" to the image repo during in roughly two weeks. Please have a look if you have any objections against this.

Here is what I have looked at already in this new function imapplymatrix.m:

  • All BIST tests pass. And there is a fair amount of useful tests included.
  • All BIST tests test for fully Matlab compatible results. (Except for the very last test, this one tests "Octave only" behavior.)
  • Coding style looks all right. (Though there might be a few lines where I missed a missing space character.)
  • The function also works fine under Octave 4.2.2 (The oldest Octave version, that the image package currently still supports.)
  • When running the demo code on the Matlab help page for this function, then I get (visually) the same results as in Matlab.
  • There is a useful help text included.
  • Maybe we should add a single semicolon at the end of line #181. (A line break in a BIST test here generates a second row in a matrix.) I can do this AFTER pushing.
  • There is some reshaping of the input data going on, in order to use the (fast) matrix multiplication of Octave to do the core calculation job. This looks good to me.
    • I have tested the run time: X=rand(1e4,1e4,3); imapplymatrix([1,1,1],X) takes roughly 3sec to compute on my laptop. Compared to 1sec to do X.*X this computation speed looks alright to me.
  • This function also supports Nd images (ndims >=4). The Matlab function can only deal with 3d images. So for n=>4 this behavior is "Octave only". But this is fine.
  • The function seems to support all the input data types that Matlab supports. (Although this is not explicitly tested for every single data type in a BIST test.)
Hartmut <hardy>
Fri 09 Nov 2018 01:12:02 AM UTC, original submission:  

Here is an implementation of a missing function imapplymatrix. MATLAB docs: https://www.mathworks.com/help/images/ref/imapplymatrix.html

I do some reshaping of X first in order to perform a vectorized multiplication with M and then similar reshaping back the output size, reshaping four times. It is not an optimal algorithm but it works.

Martin Janda <tojtoj>

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #50816:  new_imapplymatrix_V2.patch added by hardy (9KiB - text/x-patch)
file #45390:  imapplymatrix.patch added by tojtoj (8KiB - application/octet-stream)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by avinoam (Posted a comment)
  • -email is unavailable- added by hardy (Posted a comment)
  • -email is unavailable- added by hardy
  • -email is unavailable- added by tojtoj (Submitted the item)
  • -email is unavailable- added by tojtoj
  •  

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

     

    Follow 7 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2021-02-02 avinoam StatusIn Progress Done
        Open/ClosedOpen Closed
    2021-02-01 avinoam StatusNone In Progress
    2021-01-31 hardy Attached File- Added new_imapplymatrix_V2.patch, #50816
    2021-01-25 hardy Carbon-Copy- Added avinoam
    2018-11-09 tojtoj Attached File- Added imapplymatrix.patch, #45390
        Carbon-Copy- Added -email is unavailable-

    Back to the top

    Powered by Savane 3.13-d3ae.
    Corresponding source code