Tue 22 Nov 2011 11:32:20 PM UTC, comment #11:
Sure. This has been quite instructive for me. Sorry for taking time from your day.
Let me point out that I added a printf("debug note...") statement to the filter.cc file before building so that I could show you what I mentioned before.
Here it goes:
1) Octave 3.4.3 stable, patched with the patch in this thread (the error comes out, and please observe si_dims(0)).
octave:1> version
ans = 3.4.3
octave:2> y = filter(ones(1,6)/6,1,randn(1000,1),zeros(5,1));
debug note: si_dims(0) = 1
error: filter: first dimension of SI must be of length max (length (a), length (b)) - 1
octave:2> y = filter(ones(1,6)/6,1,randn(1000,1),zeros(1,5));
debug note: si_dims(0) = 1
error: filter: first dimension of SI must be of length max (length (a), length (b)) - 1
2) Octave 3.4.3 stable, with the patch and the extra fix I had mentioned (charm):
octave:1> version
ans = 3.4.3
octave:2> y = filter(ones(1,6)/6,1,randn(1000,1),zeros(5,1));
debug note: si_dims(0) = 1
octave:3> y = filter(ones(1,6)/6,1,randn(1000,1),zeros(1,5));
debug note: si_dims(0) = 1
3) Octave 3.5.0+ from Mercurial, built a few minutes ago (note the change in si_dims(0), that's why the code in comment 8 worked for you):
octave:1> version
ans = 3.5.0+
octave:2> y = filter(ones(1,6)/6,1,randn(1000,1),zeros(5,1));
debug note: si_dims(0) = 5
octave:3> y = filter(ones(1,6)/6,1,randn(1000,1),zeros(1,5));
debug note: si_dims(0) = 5
So yes, the new version will have a fixed filter() =). But there are other things involved, such as the rare behavior over SI (transposed, not transposed, ???).
I guess it is settled, right?
Thanks again
|
Tue 22 Nov 2011 07:59:39 PM UTC, comment #8:
Hello again,
I've been looking through this for the morning and I have a bunch of information to share. First, let me confirm that the file filter.cc that comes with the current 3.4.3 source download does not have the patch described in this thread. So whoever builds from this source will encounter the filter() problem.
Second, I added some debugging code to filter.cc and noticed that it doesn't matter if the SI parameter in the function call is a row or a column vector (it is expected to be a column vector), it always shows up in filter.cc as a row vector (!!), and therefore si_dims(0) in line 93 is ALWAYS one. That makes the test fail right there, because the state vector should be of length ab_len-1, which is the value of si_dims(1) when the function is called with the correct arguments. I changed that single index and the function works as expected. Maybe I am overlooking errors further down in the code, but my tests have run well.
Before transcribing some of my test code, may I ask why is it that SI ends up being a row vector no matter what? I have little knowledge of the calling process in octave, but it is clear that SI is changed.
The test code that follows filters a random signal in two different ways: first, it filters the entire vector with zero initial state values; second, it filters the vector in halfs, passing the state vectors from call to call. Differences in the result should be negligible or zero and so they are:
octave:3> x = randn(1000,1);
octave:4> b = [0.2 0.4 0.2]; a = 1; si = zeros(2,1);
octave:5> y = filter(b,a,x,si);
octave:6> [y1,sf] = filter(b,a,x(1:500),si);
octave:7> y2 = filter(b,a,x(501:1000),sf);
octave:8> max(abs((y-[y1;y2])))
ans = 0
To corroborate the fact that SI is always made into a row vector, the call:
octave:12> y = filter(b,a,x,si);
works the same (with no error messages) as
octave:13> y = filter(b,a,x,si');
after the change I propose. So, what do you think ?
Best,
Oscar
|
Thu 23 Jun 2011 07:19:02 AM UTC, comment #1:
I have tried that change, and this seems to work:
It now returns the correct result. It passes all the tests and the examples in bug #32741
Regards,
Seb
--- filter.cc.original 2011-06-22 21:12:35.375952955 +0200
+++ filter.cc 2011-06-23 08:57:30.000000000 +0200
@@ -98,12 +98,6 @@
return y;
}
- if (si_dims.length () != x_dims.length ())
- {
- error ("filter: dimensionality of SI and X must agree");
- return y;
- }
-
octave_idx_type si_dim = 0;
for (octave_idx_type i = 0; i < x_dims.length (); i++)
{
@@ -113,11 +107,6 @@
if (x_dims(i) == 1)
continue;
- if (si_dims(++si_dim) != x_dims(i))
- {
- error ("filter: dimensionality of SI and X must agree");
- return y;
- }
}
|
Thu 23 Jun 2011 06:50:59 AM UTC, original submission:
Hi,
a = ones(2,1,3,4);
a(1,1,:,:) = [1 2 3 4; 5 6 7 8; 9 10 11 12];
y = filter([1 1 1],1,a,[],3);
y(:)'
% That generates an error:
% error: filter: dimensionality of SI and X must agree
%
% It should return:
%
% ans =
% Columns 1 through 17
% 1 1 6 2 15 3 2 1 8 2
% 18 3 3 1 10 2 21
% Columns 18 through 24
% 3 4 1 12 2 24 3
%
That affect the latest dev but also 3.2, 3.4. (Don't know about earlier versions)
Regards,
Seb
|