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

 
 

patch #9849: [octave forge] (image) New function integralImage3

Submitter:  Avinoam Kalma <avinoam>
Submitted:  Wed 04 Sep 2019 08:18:07 PM UTC
   
 
Category:  Forge : new function Priority:  5 - Normal
Status:  Done Privacy:  Public
Assigned to:  carandraug Open/Closed:  Closed
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Mon 16 Dec 2019 05:13:23 AM UTC, comment #6: 
Avinoam Kalma <avinoam>
Group Member
Mon 16 Dec 2019 05:02:44 AM UTC, comment #5: 

Thanks for the review, Hartmut.
I have pushed the fixed version http://hg.code.sf.net/p/octave/image

closing.

Avinoam Kalma <avinoam>
Group Member
Sun 15 Dec 2019 09:14:25 PM UTC, comment #4: 

I have re-tested the function integralImage3.m in the patch of comment #3. (I'll call this version of the m-file V2).

There were still some spaces not following the coding style. I've fixed the ones I found. And there was even one space too much that made the last unit test fail. That is now also fixed in my new version (V3) that I'll attach here as an m-file.

Once my last changes (especially concerning the failing unit test) are included, I think this patch is now ready for pushing to the image repo (default branch).





(file #48069)

Hartmut <hardy>
Tue 03 Dec 2019 05:07:17 AM UTC, comment #3: 

Thanks for the extra tests.
Please review the updated changeset


(file #47970)

Avinoam Kalma <avinoam>
Group Member
Mon 02 Dec 2019 09:19:51 PM UTC, comment #2: 

Thank you very much for the review @Hartmut.
I will make a new changesset.

Avinoam Kalma <avinoam>
Group Member
Sat 19 Oct 2019 07:26:43 PM UTC, comment #1: 

I have had a close look at the attached file integralImage3 in comment #0. It looks fine to me.

Some minor comments:

  • I am unsure about the english grammar in the error message "I should be at most 3-dimensional image"


  • I have looked at the coding (there isn' too much), this looks fine to me.


  • I have tested the code examples on the Matlab help page, those produce identical results as this Octave implementation.


  • All tests included into this Octave file do pass.


  • Regarding coding style: There are some missing spaces after Octave commands in the test code. E.g. "class(J)" or "uint8(10)".


  • I have also done my own tests with the following code. I got the same results as with an online Matlab version. Feel free to add those examples as additional tests, but I think it would also be fine to leave the tests as they are, there are enough relevant tests, already.



# test of 3d input image:

K = magic (8);
K = reshape (K, [4 4 4]);

L = integralImage3 (K);

L1_ML = zeros (5);
L2_ML = [0 0 0 0 0;
   0 64 96 98 132;
   0 73 146 203 260;
   0 90 212 316 388;
   0 130 260 390 520];
L3_ML = [0 0 0 0 0;
  0 67 134 197 260;
  0 130 260 390 520;
  0 193 386 583 780;
  0 260 520 780 1040];
L4_ML = [0 0 0 0 0;
  0 127 222 291 392;
  0 203 406 593 780;
  0 287 606 903 1168;
  0 390 780 1170 1560];
L5_ML = [0 0 0 0 0;
  0 134 268 394 520;
  0 260 520 780 1040;
  0 386 772 1166 1560;
  0 520 1040 1560 2080];
L_ML = cat(3, L1_ML, L2_ML, L3_ML, L4_ML, L5_ML);

assert (L, L_ML);

# test of 2d input image:
X = ones(3);
Y = integralImage3(X);
Y_ML = zeros(4,4,2);
Y_ML(:,:,2) = [0 0 0 0; 0 1 2 3; 0 2 4 6; 0 3 6 9];

assert(Y, Y_ML);


So in my opinion this new function integralImage3 could be added to the image package.

Could you as a next step generate a patch file, Avinoam? Including the changes to NEWS and other bookkeeping files?

Hartmut <hardy>
Wed 04 Sep 2019 08:18:07 PM UTC, original submission:  


Please review

Avinoam Kalma <avinoam>
Group Member

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #48069:  integralImage3_V3.m added by hardy (4KiB - text/x-objcsrc)
file #47970:  integralImage3.cs added by avinoam (5KiB - text/plain)
file #47453:  integralImage3.m added by avinoam (3KiB - 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 hardy (Posted a comment)
  • -email is unavailable- added by avinoam (Submitted the item)
  • -email is unavailable- added by avinoam
  • -email is unavailable- added by avinoam
  •  

    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
    2019-12-16 avinoam StatusNone Done
        Open/ClosedOpen Closed
    2019-12-15 hardy Attached File- Added integralImage3_V3.m, #48069
    2019-12-03 avinoam Attached File- Added integralImage3.cs, #47970
    2019-09-04 avinoam Attached File- Added integralImage3.m, #47453
        Carbon-Copy- Added carandraug
        Carbon-Copy- Added hardy

    Back to the top

    Powered by Savane 3.13-d3ae.
    Corresponding source code