Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Comparison failure in text_axes:test_phase_spectrum_freqs #5525

Closed
mdboom opened this issue Nov 19, 2015 · 8 comments
Closed

Comparison failure in text_axes:test_phase_spectrum_freqs #5525

mdboom opened this issue Nov 19, 2015 · 8 comments
Milestone

Comments

@mdboom
Copy link
Member

mdboom commented Nov 19, 2015

In #5307, where I am lowering our testing tolerance down to zero, I am discovering a few recent changes that slipped through because the tolerance on our tests was too high.

This test has been broken since at least v1.4.0 (before which I can't build with current Numpys).

BASELINE RESULT:

phase_spectrum_freqs-expected

ACTUAL RESULT:

phase_spectrum_freqs

Test code:

@image_comparison(baseline_images=['phase_spectrum_freqs'],
                  remove_text=True,
                  extensions=['png'])
def test_phase_spectrum_freqs():
    '''test axes.phase_spectrum with sinusoidal stimuli'''
    n = 10000
    Fs = 100.

    fstims1 = [Fs/4, Fs/5, Fs/11]

    NFFT = int(1000 * Fs / min(fstims1))
    pad_to = int(2 ** np.ceil(np.log2(NFFT)))

    x = np.arange(0, n, 1/Fs)

    y = np.zeros(x.size)
    for i, fstim1 in enumerate(fstims1):
        y += np.sin(fstim1 * x * np.pi * 2) * 10**i
    y = y

    fig = plt.figure()
    ax1 = fig.add_subplot(3, 1, 1)
    ax2 = fig.add_subplot(3, 1, 2)
    ax3 = fig.add_subplot(3, 1, 3)

    spec1, freqs1, line1 = ax1.phase_spectrum(y, Fs=Fs, pad_to=pad_to,
                                              sides='default')
    spec2, freqs2, line2 = ax2.phase_spectrum(y, Fs=Fs, pad_to=pad_to,
                                              sides='onesided')
    spec3, freqs3, line3 = ax3.phase_spectrum(y, Fs=Fs, pad_to=pad_to,
                                              sides='twosided')

    ax1.set_xlabel('')
    ax2.set_xlabel('')
    ax3.set_xlabel('')
    ax1.set_ylabel('')
    ax2.set_ylabel('')
    ax3.set_ylabel('')

@toddrjen: You appear to have written the test. Any thoughts?

@mdboom mdboom added this to the next major release (2.0) milestone Nov 19, 2015
@dopplershift
Copy link
Contributor

Looks to me like the original result had the phase wrap around pi, and that's not in the current result. Hard to tell without ticks though,

@mdboom
Copy link
Member Author

mdboom commented Nov 23, 2015

Just an update: I thought this might be related to a change in numpy, since that's where most of the actual calculation happens. However, going back to 1.7 (the earliest that matplotlib head compiles with), the result is all the same.

I'm leaning on just updating the baseline image to reflect the new result if I don't hear otherwise.

@dopplershift
Copy link
Contributor

No argument here. Minus the wrap (which is going to depend on implementation details of the calculation), the images do appear to match still.

@tacaswell
Copy link
Member

👍 to updating the images. This is probably an API change in mlab, but no one has complained about that yet so we should probably let it go.

@QuLogic
Copy link
Member

QuLogic commented Dec 3, 2015

I tried out c89ce08 where the test image was added (against NumPy 1.8.2), but it is also incorrect there. In fact, I went back a little further to 7fc90de where the phase_spectrum plotting method was added and it also does not produce the baseline image.

@QuLogic
Copy link
Member

QuLogic commented Dec 3, 2015

It also fails against NumPy 1.6.2, since you thought it might be related.

@mdboom
Copy link
Member Author

mdboom commented Dec 3, 2015

Ok. All signs point to just updating the baseline image on this one then. Thanks for the extra investigation. I'll probably just update the image as part of #5307.

@mdboom
Copy link
Member Author

mdboom commented Dec 10, 2015

Closed by #5307.

@mdboom mdboom closed this as completed Dec 10, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants