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

BUG #1947: isofill does not handle out of bounds levels correctly. #2027

Merged
merged 3 commits into from Jun 21, 2016

Conversation

Projects
None yet
2 participants
@danlipsa
Contributor

danlipsa commented Jun 12, 2016

When smallest level is bigger than min scalar value or biggest level
is smaller than max scalar value isofill creates the wrong image.

@danlipsa

This comment has been minimized.

Contributor

danlipsa commented Jun 12, 2016

@doutriaux1 @aashish24 Please review. I did not add a new test here because we already had tests for this but they had wrong baselines.
New baselines in:
CDAT/uvcdat-testdata#143

@danlipsa

This comment has been minimized.

Contributor

danlipsa commented Jun 12, 2016

@doutriaux1 The confusing part here was that we color by the cell data generated by vtkBandedPolyDataContourFilter. These are the bands from 0 to len(l) - 1. But we can also color by the scalar in the original data - this is point data: this is what you've done in your PR, so that is why you had the fuzzy images. Your PR looked correct, but I did not understand initially how this was working before your change. I forgot about the bands generated by vtkBandedPolyDataContourFilter.

@danlipsa danlipsa force-pushed the isofill-interval branch from c6ef91b to 0c487ee Jun 12, 2016

@danlipsa danlipsa changed the title from BUG #1946: isofill does not handle out of bounds levels correctly. to BUG #1947: isofill does not handle out of bounds levels correctly. Jun 12, 2016

@doutriaux1

This comment has been minimized.

Member

doutriaux1 commented Jun 13, 2016

@danlipsa did you also test the last example I posted, I don't think this fixes it.

import cdms2
import vcs

x=vcs.init()
f=cdms2.open("HadSST1870to99.nc")
s=f("sst")
print s.min(),s.max()
iso=x.createisofill()
iso.levels=[-15,-14,-13,-12,-11,-10,-9,-8,-7,-6,35]
x.plot(s,iso)
@danlipsa

This comment has been minimized.

Contributor

danlipsa commented Jun 13, 2016

@doutriaux1 I run a selection in ParaView for all cells with scalar >= -6. This selects most cells - note ParaView does not show blanked cells because I save polygonal data which does not support blanking.

isofill-interval

This is the image generated by uvcdat - if you look carefully you'll see different colors at the top.
isofill-interval

So, I think the image is correct - it looks strange because most values are between -6 and 35.

@danlipsa

This comment has been minimized.

Contributor

danlipsa commented Jun 13, 2016

@doutriaux1 Actually, I think you are right, there is something going on here. Some black (the part that is shown blue here) should have been white. See the following image:
isofill-interval

@doutriaux1

This comment has been minimized.

Member

doutriaux1 commented Jun 13, 2016

@danlipsa yes that is odd. But at least the levels seem to be picked up correctly (on your first plot). I still don't see how it's possible from the code, but it seems to work.

@doutriaux1

This comment has been minimized.

Member

doutriaux1 commented Jun 13, 2016

@danlipsa I merged conda_build into your branch (locally) and was able to test it quickly, it seems to work ok. Not sure about the black bit though as you mentioned.

@danlipsa danlipsa force-pushed the isofill-interval branch 4 times, most recently from 0f26daa to e132a32 Jun 15, 2016

@danlipsa

This comment has been minimized.

Contributor

danlipsa commented Jun 15, 2016

@doutriaux1 @aashish24 Please review. This solves all problems I know of.

BUG #1947: isofill does not handle out of bounds levels correctly.
When smallest level is bigger than min scalar value or biggest level
is smaller than max scalar value isofill creates the wrong image.

Also, out of range (white color) was shown black.

@danlipsa danlipsa force-pushed the isofill-interval branch from e132a32 to 7211477 Jun 15, 2016

@doutriaux1

This comment has been minimized.

Member

doutriaux1 commented Jun 20, 2016

@danlipsa looks like it's working now. Thanks. But why do we do a pointtocell ?
The reason I'm asking is that I was hoping we could draw isofill/isolines on unstructured grid, even if they did not have vertices information, simply using a point dataset.

@danlipsa

This comment has been minimized.

Contributor

danlipsa commented Jun 20, 2016

@doutriaux1 putMaskOnVTKGrid needs to draw black (masked) any cell that has any points hidden. That is why I need pointtocell.

You won't be able to do isolines/isofill for a point dataset. Both need interpolation which only works if you have cells - topology information. Otherwise the algorithm won't know which points to use for interpolation.

@doutriaux1

This comment has been minimized.

Member

doutriaux1 commented Jun 20, 2016

@danlipsa I'm attaching a sample zipped nc file, that has coordinate info (lat/lon) but no bounds info (in CF format), vcs cannot plot it, but it should be able to, since we know the point data (lat/lon). It's definitely something for 3.0 anyway, just wanted to ask you about it now before I forget.
add.zip

@doutriaux1

This comment has been minimized.

Member

doutriaux1 commented Jun 20, 2016

@danlipsa I updated the baselines, it's good to be merged in as far as I'm concerned. If it's good with you as well, go ahead and merge please.

@danlipsa danlipsa merged commit 3cf4518 into master Jun 21, 2016

0 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details
continuous-integration/travis-ci/push The Travis CI build failed
Details

@danlipsa danlipsa deleted the isofill-interval branch Jun 21, 2016

@danlipsa danlipsa referenced this pull request Jun 30, 2016

Closed

isofill vcs bug #1947

@doutriaux1 doutriaux1 referenced this pull request Sep 21, 2016

Closed

isofill broken #5

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment