(adding Carne to cc)

Changing status to "In Progress"

The Matlab result for the rangefilt command in comment #19 is the following:

    19    23    23    15     8
    20    23    23    21    14
    19    19    16    19    19
    14    21    23    23    20
     8    15    23    23    19

There are also a bunch of Matlab compatible tests for rangefilt in file #39544 (from patch #9235).

When I use the patch from comment #16 (file #39535) in rangefilt.m then I get the following result for the rangefilt command in comment #19:

   19   23   23   15    8
   20   23   23   21   14
   19   19   16   19   19
   14   21   23   23   20
    8   15   23   23   19

This is identical to the above Matlab result. So Avinoam's latest patch (file #39535) seems to generate Matlab compatible results even in this new test case. (It also passes all the Matlab compatible tests from the mentioned file #39544.)

Conclusion: No, the (patched) rangefilt.m is not completly wrong. It gives Matlab compatible results in all 14 testcases that we currently have. (The current version in the repo IS completely wrong, though.)

I am very open to deal with the padding issue (in order to better deal with different numbers of dimensions) differently. I might be a clever idea to let spatial_filtering do this padding job. (But I myself wasn't eager to implement this padding in c code.) Now that we have so many test cases available for all filtering functions that call spatial_filtering (see patch #9235) the risk is significantly lower to unintentionally damage something in this code.

Using

imdilate (im, domain) - imerode (im, domain)

also produces the correct result of rangefilt for the new test case in comment #19. (I  haven't tested the results for the other 14 testcases though.) That might be an even better way (faster?) to do a fully new implementation of rangefilt.m. But in this case we still need to fix the calling of spatial_filtering (for entropyfilt and stdfilt) and the padding issue for  the remaining filter functions (probably entropyfilt, stdfilt, medfilt2, ordfilt2, maybe more).

The error here is the same as what happens in stdfilt, and the same reason why we get an error on medfilt2 and ordfiltn.  And they all call _spatial_filtering_ which is the function that requires input to have the same dimension.  I would prefer to fix _spatial_filtering_ instead to be smarter about it (there's no reason to require the same number of dimensions) than to replicate the padding in all those functions.

But on the specific case of rangefilt, I think that the function is completely wrong. From reading the documentation, I would guess that we could just use:

imdilate (im, domain) - imerode (im, domain)

But the results are different.  Reading the Matlab documentation suggests me the same so I think that rangefilt is just giving completely wrong values:

octave-cli-4.3.0+:16> a = magic(5)
a =

   17   24    1    8   15
   23    5    7   14   16
    4    6   13   20   22
   10   12   19   21    3
   11   18   25    2    9

octave-cli-4.3.0+:17> rangefilt(a, ones(3)) # this looks completely wrong
ans =

   38   34   23   21   18
   37   23   23   25   20
   23   19   22   38   38
   15   22   41   34   32
   19   22   41   34   24

See that the center point of the 5x5 matrix, is not the max - min of the points on that point.

I have tested Avinoam's third patch (file #39535):

• it solves the original issue of this bug report
• it adds (passing) tests for the mentioned (Matlab compatible) test cases of this bug report
• it doesn't give any spurious console outputs any more

Thanks for the patch Avinoam. I would be happy to see this one applied to the image repository.

(I will now soon prepare another patch to add more Matlab compatible tests to rangefilt and other related filtering functions. It will appear in the patch tracker, not the bug tracker.)

Ping. Any news here?

(I have generated a bunch of Matlab compatible tests for rangefilt, stdfilt, entropyfilt, medfilt2 and ordfilt2, which I would like to propose as a new patch on the patch tracker, after this rangefilt issue has been fixed.)

Sorry, I think _spatial_filtering_.cc is innocent and works correctly in this case. We are just mis-using it in rangefilt.m.

When I read the help string of ordfiltn.m (to learn about the fourth parameter S in the call to _spatial_filtering_.cc), then I read: "The optional variable S is a matrix of size(DOMAIN). Values of S corresponding to nonzero values of domain are added to values obtained from A when doing the sorting."

So we should probably use only 0s as S in the call to _spatial_filtering_.cc, but currently we are calling it with S = I, the code in ragefilt.m is currently this:

retval = __spatial_filtering__ (I, domain, "range", I, 0);

When I change this line in rangefilt.m in the following way:

retval = __spatial_filtering__ (I, domain, "range", zeros(size(I)), 0);

then I do get the correct result from the code in comment #10.

So I would suggest (after reviewing this) to

• use Avioams patch from comment #8
• add the mentioned semicolon to his patch
• change the fourth parameter in the single code line mentioned in this comment
• add some new tests, e.g. taken from comment #10 and #0.

@Avinoam: Could you review this and prepare a new cset?

• What does "K = rangefilt(R)" give in Matlab (with the script from comment #9)?
  • This is ment to answer the question: Are the results of rangefilt.m (with simple 2d input images) currently Matlab compatible at all?

• I have the suspicion that our current Octave implementation of rangefilt just gives results that are by +1 too big for this input value.
  • The Matlab doc says about rangefilt: "J = rangefilt(I) returns the array J, where each output pixel contains the range value (maximum value − minimum value) of the 3-by-3 neighborhood around the corresponding pixel in the input image I."
  • If the above test gives the expected result "K == J(:,:,1)" then our Octave results are abviously too big by +1 in this case. The difference between the max value and the min value in a 3x3 region of R is 2, and not 3. (See for example the center pixel, min=2, max=4.)
  • This would be a new bug. Presumably in _spatial_filtering_.cc
  • A minimal test code for debugging this new bug is for example this:

X=[0 1 0;0 0 2;0 0 0]
Y=rangefilt(X)
Y(2,2)   % should be 2 (=2-0), not 3

I've tested Avinoam's patch "rangefilt2.diff" from comment #8:

• There is a semicolon misssing after the new code line with padarray.

• The patched version of rangefilt.m still supports Nd input images. (see comment #6).

• The example (with a full white rgb-color input image to rangefilt.m ) from comment #0 works fine with this patch applied. But to judge if this usecase of ndims(I) > ndims(domain) would work fine in general, I would like to know the Matlab result of the following:

R = [1:5] .* ones(5,1);
G = R+1;
B = R*2;
I = cat(3, R, G, B)
J = rangefilt(I)

• The opposite usecase (mentioned at the end of comment #6) is not dealt with, yet. Carne suggested "If the domain has more dimensions, then the extra dimensions can be removed." But I would like to argue that this usecase is less realistic then my original one, discussed above. The original issue mostly arises because the default value of "domain" is 2d (even with images of higher dimension).

So if the patched version of rangefilt also gives Matlab compatible results on the new test code above, I would suggest to commit this (sligthly corrected) patch to the image repo, close this issue, and do the total rewrite of rangefilt (using imdilate) lateron.

Retagging release from 4.0.0-rc4 to 4.0.0.

What do you think about the following diff file?
It supports 2 and 3 dimensional images (and NOT n-dimensional like
Matlab).
The diff is wrt default branch (and not the stable).




(file #34100)

Oops, problem with the verbatim line.
(there should be an option to view the comment before submitting it)

The error message is from the line

    idx {k} = (even (k)+1):size (I, k);

even has size of 2, but k goes from 1 to 3.
It seeme that there is a confusion between the dimensions of
the domain matrix, and the dimensions of the image matrix.

The same problem (and the same solution?) in stdfilt.m

I confirm this problem. The problem has to do how the padding is done.  The function that does all the work, _spatial_filtering_, requires things to have the same number of dimensions. A workaround for this is:

pkg load image
I = ones(10,10,3);
J = rangefilt (I, padarray (ones (3), [0 0 1]));

This could be fixed by padding the things properly inside rangefilt (see psf2otf for some tips on how to handle padding of things and arbitrary number of dimensions).  However, it seems to me that this whole function could be rewritten to use imerode and imdilate for some performance as well.