BUG #1106: wrong dataset created from nc file with decreasing lat and…#1706
BUG #1106: wrong dataset created from nc file with decreasing lat and…#1706doutriaux1 merged 4 commits intomasterfrom
Conversation
|
@sankhesh @aashish24 @doutriaux1 Please review. |
|
@danlipsa this is great! I will try it later unless someone else beats me to it. Do we have a test? |
… bounds We did not handle correctly a nc file which specifies bounds and has decreasing latitude or longitude.
|
@aashish24 No, I did not add one. Should I add just the script/data that was used to show the bug? |
|
@doutriaux1 @durack1 There are some tests failing where the test image seems correct and baseline seems wrong. Could you confirm that, for instance by looking at old baselines or running the tests with a pre-vtk uvcdat? See the boxfill tests in https://open.cdash.org/viewTest.php?onlyfailed&buildid=4116767 |
danlipsa
force-pushed
the
continents-off
branch
from
49d80ef to
658ac44
Compare
|
@danlipsa I see what you mean but I'm not certain what changes have occurred that could have caused this - maybe @chaosphere2112 can answer if @doutriaux1 is not currently available..? I wonder if some of the baselines need to be updated? |
|
I'm curious as to why the isofill tests are not failing. |
|
@danlipsa Looks like we still need to bump up the meshfill tests' threshold a bit more (they're failing on Annie; I'd say go to 40). As for the actual content of this PR, I tend to agree that the updated images look better. I'd want to wait on @doutriaux1 to comment when he gets back before merging this one, though. |
|
@danlipsa can you look into these failing tests? I see that box fills are failing as well |
|
@aashish24 see the discussion earlier in the pull request. We believe the test images are correct and the baseline are wrong. We wait for @doutriaux1 to take a look at this as well. |
|
@doutriaux1 can you look at this branch and merge it if all looks good? |
|
will retrigger now and take a look tomororw morning |
|
@danlipsa if @doutriaux1 approves / agrees that new image (test image) is better than I would suggest you or @doutriaux1 update the baselines specifically for the NH ones.. |
|
@danlipsa building now. |
|
@danlipsa did you run this test on the seaice test? I think it should fix it. In the mean time I will create a baseline update. Also if it fixes the sea-ice could you add it as a test case? (put the nc file in uvcdat-testdata repo under data ) |
|
@doutriaux1 yes, it fixes the sea ice test. @aashish24 also suggested adding a test for the sea ice and I meant to answer him: My thought is that we have all these other tests that exhibited the same faulty behavior - an empty area at the pole and all data shifted. Updating the baseline for these tests would be enough to test that we maintain the correct behavior. Adding another test for sea ice would not test anything new. What do you guys think? |
|
@danlipsa the other tests while exhibiting this issue were not meant to test this though, so I think it's worth a dedicated test. |
|
on that note @aashish24 the projection is still wrong.... |
@doutriaux1 are you talking about aeqd projection? |
png ones did not clean up after themselves click_labels wasn't opening at proper size box_custom wasn't retrning a failure code when failing
|
updated baselines at: CDAT/uvcdat-testdata#89 |
|
@danlipsa when you added the test if these pass happily then let's merge. |
BUG #1106: wrong dataset created from nc file with decreasing lat and…
|
yes look at the baseline they are "cut" at the greenwich meridian, but that's another |
|
@doutriaux1 @aashish24 OK, I'll add a test for the sea ice. |
|
+1 @danlipsa |
|
@doutriaux1 The aeqd projection rendering issue is #1462 |
|
One thing that is strange to me is why did none of the |
|
@sankhesh I think isofill use pointData rather than cellData. |
|
Yes, but this change does not have anything to do with the representation. As far as I can tell, it just changes the data extents. |
6 participants
… bounds
We did not handle correctly a nc file which specifies bounds and
has decreasing latitude or longitude.