Hello all,
I think I have fixed two bugs in the contouring code (src/cntr.c):
1) inconsistent behaviour in how contour and contourf handle saddle
grid squares, and
2) incorrect handling of masked regions in filled contour plots.
Attached is a gzipped tar file containing an explanation of the bugs
and what I've done, as well as an svn diff against HEAD and small
example scripts to demonstrate the bugs before and after the fixes.
I have tested the fixes for the cases I normally come up with, but
this is by no means exhaustive.
Ideally it would be good if someone who is more familiar with cntr.c
than I am could take a look at what I've done and see if it is OK.
Ian Thomas
ianthomas23@...564...
contourbugs.tar.gz (120 KB)
Ian Thomas wrote:
Hello all,
I think I have fixed two bugs in the contouring code (src/cntr.c):
1) inconsistent behaviour in how contour and contourf handle saddle
grid squares, and
2) incorrect handling of masked regions in filled contour plots.
Attached is a gzipped tar file containing an explanation of the bugs
and what I've done, as well as an svn diff against HEAD and small
example scripts to demonstrate the bugs before and after the fixes.
I have tested the fixes for the cases I normally come up with, but
this is by no means exhaustive.
Ideally it would be good if someone who is more familiar with cntr.c
than I am could take a look at what I've done and see if it is OK.
Ian,
I will take a look, and commit the changes if things look OK. I suspect Mike D. and I are the most familiar with this code (apart from the original author--and I don't know who that is); both of us have tried unsuccessfully to solve the masked region problem. If you have solved these problems, that's fantastic!
Eric
Ian Thomas wrote:
Hello all,
I think I have fixed two bugs in the contouring code (src/cntr.c):
1) inconsistent behaviour in how contour and contourf handle saddle
grid squares, and
2) incorrect handling of masked regions in filled contour plots.
Ian,
I have applied your patch and modified contourf_demo slightly to illustrate interior masking. Thanks very much for the beautiful work! These contour bugs that you fixed were major mpl problems--general embarrassments, and specific impediments to my own applications, since the data sets I work with often have masked interior regions. (And, I also use line contours on top of filled contours, so the saddle-point decision fix helps as well.)
It occurs to me that there might be a nice refinement: when following a masked boundary, how hard would it be to cross the single-cell gap diagonally instead of proceeding step-wise along the boundary? In the case of the circular masked region that I added to the contourf_demo, this would simply smooth out the boundary of that region.
Eric
···
Attached is a gzipped tar file containing an explanation of the bugs
and what I've done, as well as an svn diff against HEAD and small
example scripts to demonstrate the bugs before and after the fixes.
I have tested the fixes for the cases I normally come up with, but
this is by no means exhaustive.
Ideally it would be good if someone who is more familiar with cntr.c
than I am could take a look at what I've done and see if it is OK.
Ian Thomas
ianthomas23@...564...
------------------------------------------------------------------------
------------------------------------------------------------------------------
Throughout its 18-year history, RSA Conference consistently attracts the
world's best and brightest in the field, creating opportunities for Conference
attendees to learn about information security's most important issues through
interactions with peers, luminaries and emerging and established companies.
http://p.sf.net/sfu/rsaconf-dev2dev
------------------------------------------------------------------------
_______________________________________________
Matplotlib-devel mailing list
Matplotlib-devel@lists.sourceforge.net
matplotlib-devel List Signup and Options
Eric Firing wrote:
Ian,
I have applied your patch and modified contourf_demo slightly to illustrate
interior masking. Thanks very much for the beautiful work! These contour
bugs that you fixed were major mpl problems--general embarrassments, and
specific impediments to my own applications, since the data sets I work with
often have masked interior regions. (And, I also use line contours on top
of filled contours, so the saddle-point decision fix helps as well.)
I'm glad my contribution passes the quality control checks!
It occurs to me that there might be a nice refinement: when following a
masked boundary, how hard would it be to cross the single-cell gap
diagonally instead of proceeding step-wise along the boundary? In the case
of the circular masked region that I added to the contourf_demo, this would
simply smooth out the boundary of that region.
Eric
I think it would be fairly easy to do half a solution to this, but
difficult to do it properly. It would be easy to change the
edge_walker function to miss out a grid point when, for example,
moving clockwise around a masked region from an i-edge to a j-edge.
But what should happen in the situations when a contour level
intersects one of those two edges: either (a) do nothing, or (b) still
do the diagonal cut-off. The do nothing option (a) is easy (!) but
means that there will be a mixture of diagonal cut-offs and stepwise
changes, which won't look particularly elegant, whereas (b) will mean
some pretty serious rewriting as the contouring code will have to deal
with these diagonal edges for both contour lines and filled contours,
and there will have to be some slightly arbitrary interpolation from
the grid z-values to these diagonal edges. So the answer to your
question is "difficult but doable".
My preference is to leave it as it is, as the current blocky solution
is what I expect to see. But I am happy to take a look at it if
you/others think it is a good idea.
On the subject of contouring masked grids, I sometimes want to specify
which grid squares are masked rather than which grid points, i.e. for
a grid of nx by ny points I want to specify a mask of (nx-1) by (ny-1)
squares. I've discovered that cntr.c uses such a square mask,
creating it from the incoming point mask. It would therefore be easy
to add support for such a grid by changing the python front end to
pass it in. Is this a good idea and would this be useful to others,
or am I being overly simplistic?
Ian
P.S. Eric, I see that you work with Kelvin Richards - he was my PhD
supervisor many years ago. Small world!
Ian,
I hit a bug (segfault) in cntr.c that is likely related to your changes. It is ID 2956378 in the tracker. Sample script and data file are there. Will you be able to take a look soon?
(Not sure whether the following link will work for you.)
https://sourceforge.net/tracker/?func=detail&aid=2956378&group_id=80706&atid=560720
Thanks.
Eric
Eric,
It appears to be caused by an infinite loop, and may well be due to my
changes. I'll take a look on Wednesday and get back to you.
Ian
···
On 22 February 2010 01:07, Eric Firing <efiring@...229...> wrote:
Ian,
I hit a bug (segfault) in cntr.c that is likely related to your changes. It
is ID 2956378 in the tracker. Sample script and data file are there. Will
you be able to take a look soon?
(Not sure whether the following link will work for you.)
https://sourceforge.net/tracker/?func=detail&aid=2956378&group_id=80706&atid=560720
Thanks.
Eric
Eric,
I hit a bug (segfault) in cntr.c that is likely related to your changes. It
is ID 2956378 in the tracker.
Attached is a patch file with a fix for this bug. I've also included
a minimal test file to demonstrate the behaviour before and after the
fix, along with a brief explanation which I can expand upon if you
wish.
Ian
patch_2956378.diff (577 Bytes)
minimal_2956378.py (979 Bytes)
Ian Thomas wrote:
Eric,
I hit a bug (segfault) in cntr.c that is likely related to your changes. It
is ID 2956378 in the tracker.
Attached is a patch file with a fix for this bug. I've also included
a minimal test file to demonstrate the behaviour before and after the
fix, along with a brief explanation which I can expand upon if you
wish.
Ian
Ian,
Excellent! Applied in svn 8152. I also added your test case to the tracker and closed the bug. Thanks for the great work!
Eric