Tue 01 Sep 2015 03:24:22 PM UTC, comment #12:
Pushed changeset here (http://hg.savannah.gnu.org/hgweb/octave/rev/4c2e76cbdc7d).
|
Tue 01 Sep 2015 02:09:31 PM UTC, comment #11:
Looks good to me, go ahead and push that if you're ready.
I'll repurpose this bug to the fix I still need to clean up for the signal package's helper function cplxreal.
|
Tue 01 Sep 2015 01:02:20 PM UTC, comment #10:
@Mike: good point. The tolerance shouldn't be absolute--what we have now--nor should it depend only on the largest value in Z.
See the next attached changeset cpair.cset. The tolerance is now calculated on a per pair basis (Z(i)). While I was fixing this I decided to overhaul the function so there are a lot of other worthwhile changes in the cset.
(file #34787)
|
Tue 01 Sep 2015 01:26:04 AM UTC, comment #9:
I have to make a change to the signal package as well to accommodate the original bug report, but this fix to cplxpair helps with that.
Rik, do you think this change makes the pairing too lenient? The way I read the Matlab function help, it makes it sound like the tolerance is done element-wise rather than based on the max(abs(z)).
The following example shows how this can go wrong:
If the tolerance were taken from the magnitude of each element, z(3) and z(4) wouldn't be a valid complex conjugate pair.
|
Mon 31 Aug 2015 06:52:05 PM UTC, comment #8:
I tried it with my octave 4.0.0 and it fixes the reported problem for me.
Doug Stewart
|
Mon 31 Aug 2015 04:11:25 PM UTC, comment #7:
Yes, sorry for losing track, I will look at this later today.
|
Mon 31 Aug 2015 03:07:40 PM UTC, comment #6:
@Mike: Can you review my patch? If it works I'll go ahead and commit it.
|
Thu 27 Aug 2015 03:58:05 AM UTC, comment #5:
@Mike: I think the problem is that the tolerance for pairing did not scale with the inputs. If you are using numbers of order 2000, then the tolerance needs to be of order eps(2000). Instead the function was using a fixed tolerance of 100 x eps(1). Try the attached diff. It works for me for the example in comment #3. It may also fix the problem with zp2sos.
(file #34750)
|
Sun 23 Aug 2015 08:34:10 PM UTC, comment #4:
ML r2015b prerelease (empty lines removed):
|
Sun 23 Aug 2015 07:19:42 PM UTC, comment #3:
I think there is a compatibility bug with cplxpair here, but maybe also with zp2sos once that is fixed.
Example:
Can OP or someone try this in Matlab to verify that it should work?
My reading of Matlab's help on cplxpair says that this should work because the tolerance is taken relative to the magnitude of each value.
|
Sun 23 Aug 2015 07:09:11 PM UTC, comment #2:
Is there a simple example that can be used to demonstrate the issue with cplxpair? The first step is to determine whether this is a problem with code in the signal package or in core Octave.
|
Sun 23 Aug 2015 11:52:31 AM UTC, comment #1:
this work around will help you until we get it fixed.
pkg load signal
N=2;
Wp=[5976, 6605];
[Z,P,K] = butter(N,Wp,'stop','s')
p1=round(1e11*P)/1e11 # one less digit of accuracy.
[SOS,G] = zp2sos(Z,p1,K)
|
Sun 23 Aug 2015 10:41:05 AM UTC, original submission:
Hi,
The following code produces an error in the function zp2sos. Note that it is working with Matlab and also if the filter type is ellip.
Thank you
|