Faster image comparison decorator #97
Conversation
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 up multiple times.
|
I also want to point out a cool script I found for measuring peak memory usage of a process on Linux: |
|
Quick note, on my computer, the Inkscape process for one of the conversions (I forget which) eats up a lot of memory (several hundreds). In the current form, at least this memory gets freed after the conversion is done. Will the memory get cleared after each conversion by Inkscape? |
|
I didn't detect any additional peak memory usage in Inkscape this way vs. the old way. Googling reveals various bug reports about Inkscape in --shell mode, but those may be obsolete bugs. I think this requires some testing with various Inkscapes on various systems to ensure it's an ok approach. (I'm running Inkscape 0.48.1 on Fedora 14, FWIW) |
| @classmethod | ||
| def setup_class(cls): | ||
| cls.original_rcParams = {} | ||
| cls.original_rcParams.update(matplotlib.rcParams) |
There was a problem hiding this comment.
shouldn't this be rcParamsDefault - since test may be launched after the user starts up e.g. ipython with a profile which modifies the rcParams - making test incompatible?
also, is there a reason to not just do it in one line, instead of two?
cls.original_rcParams = matplotlib.rcParamsDefault.copy()
same for the orignal_units_registry below
There was a problem hiding this comment.
Good points on both. I think I was confused by the restoration phase -- which does need the .clear and .update because we can't replace the underlying rcParams object which is referenced all over the matplotlib source code.
There was a problem hiding this comment.
Ah -- on your first point: it turns out we can't use rcParamsDefault since the test infrastructure overrides certain things in a setup() function. See lib/matplotlib/tests/init.py.
def setup():
use('Agg', warn=False) # use Agg backend for these tests
# These settings *must* be hardcoded for running the comparison
# tests and are not necessarily the default values as specified in
# rcsetup.py
rcdefaults() # Start with all defaults
rcParams['font.family'] = 'Bitstream Vera Sans'
rcParams['text.hinting'] = False
Maybe we should call this function from the teardown part of the decorator? Then everything gets reset to this.
|
Great job, Michael - this looks like it was a lot more work than my duct-tape fix ;) I like the DRYer elimination of savefig everywhere, since the name is already passed to the decorator. |
|
oh, damn it! not this again - i suck. |
|
No worries. These github guys have made development too easy! :) I just pushed a couple of commits to this branch that address some of the comments here, though -- hopefully there's an easy way to apply them, too. |
|
in my defense, the behavior of the merge button changed only two days ago: it used to only provide you with instructions on how to do the merge, not perform it for you. |
|
Yeah -- it's a somewhat misguided feature, IMHO. It doesn't give an opportunity to test the change -- and it no longer provides the really useful instructions on how to do it manually. |
|
damn, looks like a current bug: |
|
I was able to open a new pull request, though, so not so bad, excepting losing the existing comments. It's here: |
Enh image plotting
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: