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

Update vtk proj4 9 2 #1827

Merged
merged 6 commits into from Mar 9, 2016

Conversation

Projects
None yet
3 participants
@danlipsa
Contributor

danlipsa commented Feb 12, 2016

This is using a VTK branch not commited into master so do not merge yet.

@danlipsa

This comment has been minimized.

Contributor

danlipsa commented Feb 12, 2016

@danlipsa

This comment has been minimized.

Contributor

danlipsa commented Feb 12, 2016

@danlipsa danlipsa force-pushed the update_vtk_proj4_9_2 branch 3 times, most recently from 0e0d84c to 7bae2ac Feb 12, 2016

@danlipsa

This comment has been minimized.

Contributor

danlipsa commented Feb 15, 2016

@doutriaux1 Is cont-int/LLNL/Linux-RH6 (FULL) working? Because it says: 'checking whether the C compiler works... no' I would think it is a problem with the machine. Are there any mac buildbots left?

@danlipsa

This comment has been minimized.

Contributor

danlipsa commented Feb 15, 2016

@aashish24 @doutriaux1 Please review. I think this is a step in the right direction with aeqd and no other projection seems worst than it was. Some of the aeqd tests look a lot better some don't. Given that aeqd was completely broken in the past I would say to merge this and do another pass through the aeqd tests in the future. I think we can improve how they look.

@danlipsa

This comment has been minimized.

Contributor

danlipsa commented Feb 15, 2016

@aashish24 We need to merge proj4 upgrade branch into VTK first before we can merge this. See
https://gitlab.kitware.com/vtk/vtk/merge_requests/1207

@doutriaux1

This comment has been minimized.

Member

doutriaux1 commented Feb 16, 2016

@danlipsa looks like xfvb needs a restart on RH6 (FULL) and bots are down after one build as usual...

@doutriaux1

This comment has been minimized.

Member

doutriaux1 commented Feb 16, 2016

@danlipsa are you using Kitware/VTK? I thought we were supposed to use UV-CDAT/VTK so we do not have to wait for Kitware/VTK.

@aashish24

This comment has been minimized.

Contributor

aashish24 commented Feb 16, 2016

@aashish24 We need to merge proj4 upgrade branch into VTK first before we can merge this. See
https://gitlab.kitware.com/vtk/vtk/merge_requests/1207

Reviewed it and +2 it. @doutriaux1 once this goes into master, we will merge master into UVCDAT/VTK:uvcdat-master.

@danlipsa

This comment has been minimized.

Contributor

danlipsa commented Feb 16, 2016

@aashish24 I get conflicts merging VTK master into uvcdat-master. I'll let you do this merge as you have managed the commits into both. @aashish24 @doutriaux1 In the future I would suggest always merge things into VTK master before we use a certain SHA into uvcdat. Having a separate branch and merging things into both VTK master and uvcdat-master can lead to conflicts and makes it more difficult to update VTK. Note that ParaView used a separate branch in the past but it got rid of it and now it uses directly a certain SHA from VTK.
If there are no changes in uvcdat that are not merged in VTK I would suggest updating uvcdat-master to VTK master.

@doutriaux1

This comment has been minimized.

Member

doutriaux1 commented Feb 16, 2016

@danlipsa

This comment has been minimized.

Contributor

danlipsa commented Feb 17, 2016

@doutriaux1 this looks like the old proj4. Make sure the VTK got updated: remove VTK-prefix and build/VTK-build

@aashish24

This comment has been minimized.

Contributor

aashish24 commented Feb 17, 2016

@aashish24 I get conflicts merging VTK master into uvcdat-master. I'll let you do this merge as you have managed the commits into both.

You got merge conflict because I pull in one fix from UV-CDAT to VTK. I will fix the conflict as its a easy one. We pretty much have all the changes in VTK master except the FFPEG one. If we keep merging changes to VTK propoer, we won't have these conflicts.

@doutriaux1

This comment has been minimized.

Member

doutriaux1 commented Feb 17, 2016

I'm pretty sure it was a fresh build, but I will double check

@danlipsa

This comment has been minimized.

Contributor

danlipsa commented Feb 17, 2016

@aashish24 OK, so after we merge everything into VTK we can just update uvcdat-master to the latest VTK isn't it?

@danlipsa

This comment has been minimized.

Contributor

danlipsa commented Feb 17, 2016

@doutriaux1 I will try running the tests on my mac as well.

@aashish24

This comment has been minimized.

Contributor

aashish24 commented Feb 17, 2016

aashish24 OK, so after we merge everything into VTK we can just update uvcdat-master to the latest VTK isn't it?

Right that is the idea. The only part thats not in VTK proper is the FFMPEG code. The reason I am afraid to bring it in VTK is because I am not 100% sure why the code was written the way it was written but somehow was working. I guess I will take the risk and push a VTK PR and see what happens. In the mean time, what I am going to do is reset UVCDAT/uvcdat-master to VTK master and bring in my change on top of it.

@danlipsa danlipsa force-pushed the update_vtk_proj4_9_2 branch from 7bae2ac to c1bb819 Mar 7, 2016

@danlipsa danlipsa changed the title from WIP: Update vtk proj4 9 2 to Update vtk proj4 9 2 Mar 7, 2016

@danlipsa

This comment has been minimized.

Contributor

danlipsa commented Mar 7, 2016

@doutriaux1 @aashish24 All tests pass on my ubuntu, also cdat-web works. I'll run the tests at home on my mac.

@doutriaux1

This comment has been minimized.

Member

doutriaux1 commented Mar 7, 2016

@danlipsa running this one more time on my mac just to be sure.

@danlipsa

This comment has been minimized.

Contributor

danlipsa commented Mar 8, 2016

@doutriaux1 I run this on my mac. Looks good besides expected (and anoying) small differences in some aeqd new images. vcs_mintics has more differences but think this is a test @chaosphere2112 just added. I added the file on linux as it was missing.

@danlipsa

This comment has been minimized.

Contributor

danlipsa commented Mar 8, 2016

@aashish24 I can remove the gdal stuff. Do need that saved in a patch file?

@aashish24

This comment has been minimized.

Contributor

aashish24 commented Mar 8, 2016

@aashish24 I can remove the gdal stuff. Do need that saved in a patch file?

Yes, thanks. A patch would work (just in case).

@danlipsa danlipsa force-pushed the update_vtk_proj4_9_2 branch from c1bb819 to d690a69 Mar 8, 2016

@danlipsa danlipsa force-pushed the update_vtk_proj4_9_2 branch from d690a69 to daf552b Mar 8, 2016

@danlipsa

This comment has been minimized.

Contributor

danlipsa commented Mar 8, 2016

@aashish24 Here is the patch:
gdal-patch.txt

@danlipsa

This comment has been minimized.

Contributor

danlipsa commented Mar 9, 2016

@doutriaux1 @aashish24 This is ready to merge!

@aashish24

This comment has been minimized.

Contributor

aashish24 commented Mar 9, 2016

LGTM 👍

doutriaux1 added a commit that referenced this pull request Mar 9, 2016

@doutriaux1 doutriaux1 merged commit 37854be into master Mar 9, 2016

3 checks passed

continuous-integration/kitware-buildbot/uvcdat-garant-linux-release/ Build done.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@doutriaux1 doutriaux1 deleted the update_vtk_proj4_9_2 branch Mar 9, 2016

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