Skip to content
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

Issue 1170 isofill mask #1171

Merged
merged 2 commits into from
Mar 26, 2015
Merged

Issue 1170 isofill mask #1171

merged 2 commits into from
Mar 26, 2015

Conversation

doutriaux1
Copy link
Contributor

No description provided.

isofill are now generated in point grids whereas the missing mapper was always using a cell2point
that led to a shift in masks
this fixes it
@doutriaux1
Copy link
Contributor Author

goes with CDAT/uvcdat-testdata#28
@chaosphere2112 @painter1 @aashish24 @dlonie please review

@alliepiper
Copy link
Contributor

@doutriaux1 Several tests are failing on my machine with this:

The following tests FAILED:
        104 - vcs_test_basic_isofill_masked (Failed)
        160 - vcs_test_basic_isofill_masked_0_proj (Failed)
        170 - vcs_test_basic_isofill_masked_-3_proj (Failed)
        180 - vcs_test_basic_isofill_masked_aeqd_proj (Failed)
        268 - vcs_test_basic_isofill_bigvalues (Failed)
        279 - dv3d_vector_test (Failed)

https://www.cdash.org/viewTest.php?onlyfailed&buildid=3745021

The kicker is that, aside from the dv3d test, the baselines are actually invalid and the new images are correct. These tests should have been failing for quite a while.

Do we have any idea how many of the valid baselines we're testing against are actually correct? It might be worth having a vcs dev/user make a pass through them and make sure that we're really testing against valid baselines in UVCDAT -- otherwise we're just masking broken code that we should be aware of.

@aashish24
Copy link
Contributor

This is interesting. I guess we are not checking / reviewing code thoroghly. The baselines indeed look wrong to me.

@alliepiper
Copy link
Contributor

As for this patch, let's get it in once the additional baselines are added to the testdata patch.

@aashish24
Copy link
Contributor

I agree @dlonie 👍

@doutriaux1
Copy link
Contributor Author

@dlonie that's VERY interesting becausethe ctest actually passed for me yesterday (which was actually disturbing) which is why I added a test to make sure we catch this. I'm very glad it fails for you because it SHOULD fail. Would you mind updating the baselines then since you already have the good updated ones.

@alliepiper
Copy link
Contributor

Sure. BTW, you can always grab the images off of a CDash failure page without regenerating them locally. They'll be identical to the ones produced during the test.

@alliepiper
Copy link
Contributor

New baselines added. @doutriaux1 Can you retest on your machine and make sure these are working for you? If it passed for you yesterday it's worth seeing what happens with the current verisons -- feel free to merge this if everything looks good on your end.

@doutriaux1
Copy link
Contributor Author

@dlonie I will retest with both old and new baseline. I "hope" I simply ran the ctest with a wrong version of uvcdat yesterday! But if not I will try to see why they pass whenthey shouldn't. Thanks for updating the baseline

@doutriaux1
Copy link
Contributor Author

@dlonie ok good, these fail for me as well with old baseline! That's reassuring, testing against new baseline and merging in if it passes.

@doutriaux1
Copy link
Contributor Author

yeo! we're good to go! merging in.

doutriaux1 added a commit that referenced this pull request Mar 26, 2015
@doutriaux1 doutriaux1 merged commit 1d1a6bf into master Mar 26, 2015
@doutriaux1 doutriaux1 deleted the issue_1170_isofill_mask branch May 14, 2015 21:43
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