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

Conda build #142

Merged
merged 6 commits into from
Jun 15, 2016
Merged

Conda build #142

merged 6 commits into from
Jun 15, 2016

Conversation

doutriaux1
Copy link
Contributor

@aashish24 @sankhesh @danlipsa @chaosphere2112 @dnadeau4 let's carefully inspect the baselines, I'm not positive why they had to be regenerated.

@doutriaux1 doutriaux1 mentioned this pull request Jun 9, 2016
@danlipsa
Copy link
Contributor

danlipsa commented Jun 9, 2016

@doutriaux1 What triggers this change? Is it the fact that uvcdat uses the VTK build through the conda receipt? What if I rebuild VTK by hand and make uvcdat use that? This is something we'll need to upgrade VTK or add/fix things there.

Is the VTK build using conda different than VTK built by hand?

@doutriaux1
Copy link
Contributor Author

@danlipsa my guess is that it has something to do with the way VTK was built. It is the uvcdat-master. It could be the png libraries that are different (conda png vs system png) I'm not positive. It seems to be fonts related mostly. I also see some difference in the markers sizes.
The VTK conda build script can be found at: https://github.com/UV-CDAT/conda-recipes/blob/master/vtk/build.sh

@doutriaux1
Copy link
Contributor Author

@danlipsa the way conda works you would have to rebuild VTK inside your conda. It might be worth trying.

@danlipsa
Copy link
Contributor

danlipsa commented Jun 9, 2016

@doutriaux1 I see -DVTK_USE_SYSTEM_FREETYPE:BOOL=ON in vtk_external.cmake and is OFF in your conda receipe. I would compare CMakeCache.txt from conda and from our cmake build and make them the same. If you need a certain option in conda you can change that in the cmake build. We should be able to have the same baselines regardless of how we build, especially on the same system.

@doutriaux1
Copy link
Contributor Author

@danlipsa ok I will try to turn off freetype in my ubuntu and see if it makes a difference.

@doutriaux1
Copy link
Contributor Author

@danlipsa this and issue #1947 are critcial for the release #1947 is a massive bug, I pinpointed the error area in my bracnh but I think it needs some kitware love. it is really really bad.

@doutriaux1 doutriaux1 merged commit a092c99 into master Jun 15, 2016
@doutriaux1 doutriaux1 deleted the conda_build branch June 15, 2016 15:31
@danlipsa
Copy link
Contributor

@doutriaux1 @aashish24 @chaosphere2112 We have a regression from the conda build:
https://github.com/UV-CDAT/uvcdat-testdata/blob/429660706c2bcf8b5dc5726c3494dcae08d835c2/baselines/vcs/test_vcs_basic_boxfill_aeqd_proj_NH.png
It seems we are using an old proj4. Do we use the conda system proj4?

@danlipsa
Copy link
Contributor

All aeqd baselines are the old ones before upgrading proj4.

@doutriaux1
Copy link
Contributor Author

@danlipsa conda states it is using:

proj4                     4.9.1                         0    defaults

VTK is built using:

        -DLIBPROJ4_INCLUDE_DIR:PATH=${PREFIX}/include \
        -DVTK_USE_SYSTEM_LIBPROJ4:BOOL=ON \
        -DLIBPROJ4_LIBRARIES:FILEPATH=${PREFIX}/lib/libproj.so \

Do you know what could have gone wrong?

@doutriaux1
Copy link
Contributor Author

Do we need 4.9.2 exactly? Is 4.9.1 bad? I will try to build our own proj4 and see if it makes a difference

@danlipsa
Copy link
Contributor

Indeed, vtk uses 4.9.2. Not sure when the aeqd fix went into proj4, but it seems that what we are using from conda is not good.

@aashish24
Copy link
Contributor

I remember that 4.9.1 had some bug with onw of the projection. Use 4.9.2 please.

@danlipsa
Copy link
Contributor

danlipsa commented Jun 16, 2016

@doutriaux1 @aashish24 What causes the big points in the line plots? They are kind of distracting.
https://github.com/UV-CDAT/uvcdat-testdata/blob/429660706c2bcf8b5dc5726c3494dcae08d835c2/baselines/vcs/test_vcs_basic_xyvsy_zero.png
Seems that the same bigger points make the scatter plots look better
https://github.com/UV-CDAT/uvcdat-testdata/blob/429660706c2bcf8b5dc5726c3494dcae08d835c2/baselines/vcs/test_vcs_basic_xyvsy_zero.png
before the points were barely visible.

@danlipsa
Copy link
Contributor

@doutriaux1 @aashish24 The vectors are also bigger - this looks better. I am wondering why? Maybe some calculation based on font was going on?
https://github.com/UV-CDAT/uvcdat-testdata/blob/429660706c2bcf8b5dc5726c3494dcae08d835c2/baselines/vcs/test_vcs_basic_vectors_-45.png

@danlipsa
Copy link
Contributor

@doutriaux1 @aashish24 This seems that the rendering for this baseline changed slightly - not sure why. Just compare this with the previous image.
https://github.com/UV-CDAT/uvcdat-testdata/blob/master/baselines/vcs/test_vcs_flipX.png

@danlipsa
Copy link
Contributor

@danlipsa
Copy link
Contributor

OK, I am done. 😄

@doutriaux1
Copy link
Contributor Author

@danlipsa looks like building agains 4.9.2 helps, see: https://open.cdash.org/viewTest.php?onlyfailed&buildid=4415031 let me know if that looks better, if so I will upload that VTK and we'll be good to go. As well as new baselines.

@doutriaux1
Copy link
Contributor Author

runnnig the full test suite now will post link (above was just aeqd)

@doutriaux1
Copy link
Contributor Author

ok it only affects aeqd full test suite at: https://open.cdash.org/viewTest.php?onlyfailed&buildid=4415069

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