Skip to content

Faster image comparison decorator #98

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

Merged
merged 11 commits into from
May 2, 2011

Conversation

mdboom
Copy link
Member

@mdboom mdboom commented Apr 27, 2011

This is a riff on the ideas in the comments on the pull requests by Paul @ivanov to reduce memory consumption when running the regression tests. See:

#96

and

#56

This is different from the other two in that it:

  1. Calls the test code to generate the figure object only once, and then calls savefig three times on the same figure -- this results in an ~ 12% speedup.
  2. Automatically closes all figures and restores rcParams and the unit registry after every test. The latter prevents some nasty side effect problems presented by some tests. This also revealed a memory leak in pyplot.close('all'). This saves massive amounts of memory vs. the current master, but it roughly equivalent to Paul's approach in pull request 96.
  3. Runs Inkscape in "shell" mode so that a single inkscape process is fired up and re-used to convert each SVG to PNG. This results in an additional ~ 32% speedup.

mdboom added 5 commits April 26, 2011 16:29
  1) Run the test itself that generates figure(s) only once, then
     savefig it to each output format.  This appears to have an approx
     2.75x speedup on run time.

  2) Cleanup better after each test:

       Close all figures
       Reset rcParams to their defaults
       Reset the unit registry to defaults

This requires the following changes to the tests themselves:

  1) The tests no longer need to call 'savefig' -- just create figures
     and the decorator will find them.

  2) Tests that don't use the image_comparison decorator should use the
     new cleanup decorator to make sure that cleanup happens.
…cape server and starting a new one to prevent memory leaks in Inkscape from becoming problematic.
…n test setup function to reset rcParams state.
@mdboom
Copy link
Member Author

mdboom commented Apr 27, 2011

This is moved over from here:

#97

@WeatherGod, @ivanov and @mdboom were participating over there.

@ivanov
Copy link
Member

ivanov commented Apr 28, 2011

hmm, i'm getting some errors in python2.5:

Failure: AttributeError ('function' object has no attribute 'im_class') ... ERROR matplotlib.tests.test_backend_svg.test_visibility.test ... ok matplotlib.tests.test_basic.test_simple ... ok matplotlib.tests.test_basic.test_simple_knownfail ... KNOWNFAIL: Test known to fail matplotlib.tests.test_basic.test_override_builtins ... ok matplotlib.tests.test_cbook.Test_delete_masked_points.test_bad_first_arg ... ok matplotlib.tests.test_cbook.Test_delete_masked_points.test_datetime ... ok matplotlib.tests.test_cbook.Test_delete_masked_points.test_rgba ... ok matplotlib.tests.test_cbook.Test_delete_masked_points.test_string_seq ... ok matplotlib.tests.test_cbook.test_is_string_like ... ok matplotlib.tests.test_cbook.test_restrict_dict ... ok matplotlib.tests.test_mlab.test_colinear_pca ... ok matplotlib.tests.test_mlab.test_recarray_csv_roundtrip ... FAIL matplotlib.tests.test_mlab.test_rec2csv_bad_shape ... ok matplotlib.tests.test_mlab.test_prctile ... ok matplotlib.tests.test_transforms.test_Affine2D_from_values ... ok Failure: AttributeError ('function' object has no attribute 'im_class') ... ERROR matplotlib.tests.test_axes.test_arc_ellipse.test ... ok matplotlib.tests.test_axes.test_arc_ellipse.test ... ok matplotlib.tests.test_axes.test_arc_ellipse.test ...

and then stalls and never finishes. Is this a nose issue? (I'm on nose 1.0.0)

@mdboom
Copy link
Member Author

mdboom commented Apr 28, 2011

Ah. I'm having issues with nose 1.0 as well. Investigating...

@mdboom
Copy link
Member Author

mdboom commented Apr 28, 2011

This latest commit seems to fix the nose-1.0.0 issue on Python 2.7. @ivanov -- do you mind kicking the tires with Python 2.5? Python 3.x will have its own set of issues, I suspect, but I'll defer that to when I merge this into the py3 branch.

@ivanov
Copy link
Member

ivanov commented Apr 28, 2011

@mdboom - I'm getting a bunch of ERROR: Failure: AttributeError ('staticmethod' object has no attribute '__func__') messages, but once I got around that with ivanov@be6af2f - it still stalls on the third matplotlib.tests.test_axes.test_arc_ellipse.test ...

@mdboom
Copy link
Member Author

mdboom commented Apr 28, 2011

It's probably hitting some sort of deadlock with the Inkscape server. I've got 0.48.1. But it also could be a difference in the behavior of the "subprocess" module in Python 2.5 vs. 2.7, or some sort of platform difference. If you revert lib/matplotlib/testing/compare.py to what is in master, does the stalling go away? If so, maybe we should defer the Inkscape server part of this pull request until we better understand how to do it in a robust way.

@ivanov
Copy link
Member

ivanov commented Apr 28, 2011

mdboom, on 2011-04-28 05:37, wrote:

If you revert lib/matplotlib/testing/compare.py to what is in master, does the stalling go away?

That did the trick, Mike! I still have a couple of failures on
that box, but I also get them on master, so I'll have to look
into that later.

I still needed to use the get(1) business, instead of
func - see https://github.com/ivanov/matplotlib/commits/faster-image-comp

mdboom added 3 commits April 28, 2011 12:46
…mage_comparison_decorator

Fix for Python 2.5 compatibility.
…er than running it as a server, which mysteriously causes a deadlock on some systems.
@ivanov
Copy link
Member

ivanov commented Apr 28, 2011

looks good now, Mike, thanks!

mdboom added a commit that referenced this pull request May 2, 2011
@mdboom mdboom merged commit fdf5fae into matplotlib:master May 2, 2011
@richbwood richbwood mentioned this pull request Dec 19, 2012
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

Successfully merging this pull request may close these issues.

3 participants