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

@doutriaux1 doutriaux1 commented Mar 26, 2015

No description provided.

doutriaux1 added 2 commits Mar 25, 2015
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

@doutriaux1 doutriaux1 commented Mar 26, 2015

@allisonvacanti
Copy link
Contributor

@allisonvacanti allisonvacanti commented Mar 26, 2015

@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

@aashish24 aashish24 commented Mar 26, 2015

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

@allisonvacanti
Copy link
Contributor

@allisonvacanti allisonvacanti commented Mar 26, 2015

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

@aashish24
Copy link
Contributor

@aashish24 aashish24 commented Mar 26, 2015

I agree @dlonie 👍

@doutriaux1
Copy link
Contributor Author

@doutriaux1 doutriaux1 commented Mar 26, 2015

@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.

@allisonvacanti
Copy link
Contributor

@allisonvacanti allisonvacanti commented Mar 26, 2015

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.

@allisonvacanti
Copy link
Contributor

@allisonvacanti allisonvacanti commented Mar 26, 2015

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

@doutriaux1 doutriaux1 commented Mar 26, 2015

@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

@doutriaux1 doutriaux1 commented Mar 26, 2015

@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

@doutriaux1 doutriaux1 commented Mar 26, 2015

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

doutriaux1 added a commit that referenced this issue Mar 26, 2015
@doutriaux1 doutriaux1 merged commit 1d1a6bf into master Mar 26, 2015
0 of 2 checks passed
@doutriaux1 doutriaux1 deleted the issue_1170_isofill_mask branch May 14, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants