patchGNU Octave - Patches: patch #8612, impyramid.m for image package

 
 

patch #8612: impyramid.m for image package

Submitter:  Avinoam Kalma <avinoam>
Submitted:  Tue 24 Feb 2015 08:12:22 PM UTC
   
 
Category:  None Priority:  5 - Normal
Status:  Done Privacy:  Public
Assigned to:  None Open/Closed:  Closed
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Wed 14 Oct 2015 05:28:33 PM UTC, comment #6: 


> When I look at the Matlab documentation for impyramid (see http://de.mathworks.com/help/images/ref/impyramid.html ) then I read "Reduction and expansion take place only in the first two dimensions."
>
> To me this says that it is alright to always do the reduction and filtering process ONLY to the dimensions 1 and 2 of any given input. The other possible dimensions should probably be kept intact during this procedure. But in Matlab this function impyramid is NOT ment to reduce an ND image along the third or fourth dimension at all!


You are right. I missed that note on the Matlab documentation. So we 're kinda limited there.

I have pushed the original function (after small style changes):

http://hg.code.sf.net/p/octave/image/rev/1f6389a88846

I then replaced the usage of imfilter with padarray and convn so it accepts arrays of any dimensions (but still only reduces/expands) the first 2.

http://hg.code.sf.net/p/octave/image/rev/460dfaf2673c

I then added a bunch of more tests.

Carnë Draug <carandraug>
Group Member
Sun 11 Oct 2015 06:25:53 PM UTC, comment #5: 

When I look at the Matlab documentation for impyramid (see http://de.mathworks.com/help/images/ref/impyramid.html ) then I read "Reduction and expansion take place only in the first two dimensions."

To me this says that it is alright to always do the reduction and filtering process ONLY to the dimensions 1 and 2 of any given input. The other possible dimensions should probably be kept intact during this procedure. But in Matlab this function impyramid is NOT ment to reduce an ND image along the third or fourth dimension at all!

For this reason I think that the proposed code in comment 4 would NOT be correct. Because it would filter along ALL present dimensions of a given input, which would be too much.

I would suggest to leave the code mostly like it is, in this respect:

case "reduce"
  im1 = imfilter (im, filt, 'replicate');
  im2 = imfilter (im1, filt', 'replicate');

The result of keeping this would be the following: As long as imfilter onyl supports n*m and n*m*3 images (as it does today), this would work fine, because impyramid does (better: should do) an input checking and only allows those types of input images. And as soon as imfilter will support Nd images, this would still work as intended: One can remove the input checking in impyramid, and then it will filter an Nd image along the first, and afterwards along the second dimension. And it would keep the other dimensions intact, because this is what imfilter will probably do if you give it for example an n*m*k*l image and in a*b filter.

Note: Currently impyramid only seems to work with n*m and n*m*3 images (as does imfilter). We should note this corretly in the documentation. Both do NOT work with n*m*k images in general.

Hartmut <hardy>
Sat 30 May 2015 03:41:25 PM UTC, comment #4: 

This is not really handling 3 dimensional images.  In such cases, it is only reducing and expanding the 1st and 2nd dimension, and leaves the 3rd dimension intact. I understand that fixing imfilter to handle those cases correctly but as it is, it cannot be accepted.  It will cause a lot of problems on the long run.

Let us suppose that I accept and release this as part of the image package.  Users will start to expect it to behave like this. Then one day we finally fix it so it handles extra dimensions correcly. All the users will need to go and change their code. That can't be.  It will be better if you just limit to 2 dimensions if you can't fix imfilter.

However, if you do it, please do it in such way that this limitation can be easily remove as soon as imfilter is able to handle more dimensions, like so:


case "reduce"
  for d = 1:nd
    im = imfilter (im, vec (filt, d), "replicate");
  endfor


Carnë Draug <carandraug>
Group Member
Thu 05 Mar 2015 04:06:28 PM UTC, comment #3: 

Here is a modified version.
answer to comment #1:

1. I could fix the borders for the 'reduce' case with the
 'replicate' keyword in the imfilter.  There is is still
difference of 1-2 gray levels in the image.
For the 'expand' case, I have corrected the image size which
was wrong, but I did not find a way to fix the border issue.
In the rest of the image, there a difference of 1-2 graylevels,
which is acceptable.
2. it seems to me that 'reduce' step is more natural, but to
be matlab compatible I have dropped the default value.
3. Done
4. It's beyond my skills for now, so I have left a FIXME comment
5. Done
6. Tabs were deleted. I did not find trailing whitespaces.
7.  Done.

Two issues remain:
1. Fix the borders in the 'expand' case
2. Support n-dimensional images

I will be glad if you will help me with the first issue. The 2nd
could (IMHO) be postponed to later stage.

Thanks!


(file #33251)

Avinoam Kalma <avinoam>
Group Member
Wed 25 Feb 2015 09:13:13 PM UTC, comment #2: 

Thanks for your comments.
I will change the routine according to your suggestions.

Avinoam Kalma <avinoam>
Group Member
Wed 25 Feb 2015 06:22:25 PM UTC, comment #1: 

Some comments:

1. the results are wrong for the image border. I will guess that the problem is the values used on the border during the convolution.  Seems like Matlab 2010b will have the following results (using your tests)


reduced =

  114  139  131  103  111
   97  122  141  111  100
  103  123  112  123  122
   47  107  134  153   94



expanded =

  115  154  185  178  150  122  105  116  138  159  158  117   78   86  112  129  133  120  103
   69   98  128  141  146  152  152  139  125  127  121   87   55   58   81  113  131  112   84
   40   54   74  100  131  167  184  157  119  104   92   64   41   44   66  100  121  103   74
   76   69   65   75   97  130  153  148  131  122  108   80   61   79  103  105   98   97   98
  120  105   88   77   78   96  121  143  155  154  140  112   98  124  143  109   74   91  123
  117  129  134  119  107  125  153  173  180  172  156  143  138  146  140   96   60   83  122
   99  139  170  157  139  156  181  188  180  164  151  154  156  140  112   81   65   84  110
  101  136  163  153  133  132  138  136  130  122  120  130  133  108   82   86   99  104  104
  103  126  143  136  116   97   81   73   73   82   94  105  105   87   78  108  138  133  116
   90  116  139  139  122   96   69   52   53   80  109  114  111  116  128  148  163  164  160
   66   99  131  140  131  109   83   62   62  102  142  144  138  154  169  164  157  169  184
   41   68   99  121  130  122  107   92   95  133  173  182  172  156  135  114  105  121  142
   21   38   64   98  124  131  127  123  129  160  194  212  199  144   82   52   48   65   85


2. You are setting the direction default to reduce.  A default makes sense when there's a value that makes more sense than the others (sometimes even changing depending on the rest of the input).  This is not the case. Could you drop the default value and require 2 nargin?

3. isgray and isrgb will check the actual numeric values. If the input is class double, it will return false for values outside [0 1]. Please only check if input isnumeric.

4. like the point above, this should work for n dimensional input and not be limited to number of dimensions (think that input may be of size [512, 789, 12, 23] and we should not be questioning that). At the moment, isrgb and isgray would fail if 3rd dimension is not 1 or 3 which would prevent this. The next weakest link would be imfilter which would not handle such thing. Could you fix imfilter to handle this?

The main trick to handle this type of things is to get a vector of dimensions, use them to create a cell array for the indices, and then turn it into a comma separated list (cell{:}) for indexing, like in


s = [16 16 16]
idx = arrayfun (@colon, ones (1, 3), s, "UniformOutput", false)
img(idx{:}) ...


5. you do not need to check ischar (direction). Simply use on the switch block. If it does not match anything, then you can use the otherwise to give a better error message (either is not a string or an invalid string value

6. please no hard tabs, only spaces. Also, could you please remove the trailing whitespace?

7. could you add your name and email to the copyright?

Carnë Draug <carandraug>
Group Member
Tue 24 Feb 2015 08:12:22 PM UTC, original submission:  


Please check, fix, add, delete etc....

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 #33251:  impyramid.m added by avinoam (6KiB - application/octet-stream)
file #33176:  impyramid.m added by avinoam (6KiB - 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)
  •  

    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 4 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2015-10-14 carandraug StatusNone Done
        Open/ClosedOpen Closed
    2015-03-05 avinoam Attached File- Added impyramid.m, #33251
    2015-02-24 avinoam Attached File- Added impyramid.m, #33176

    Back to the top

    Powered by Savane 3.13-02a9.
    Corresponding source code