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

Vtk update leak #2030

Merged
merged 2 commits into from Jun 22, 2016

Conversation

Projects
None yet
3 participants
@danlipsa
Contributor

danlipsa commented Jun 17, 2016

No description provided.

@aashish24

This comment has been minimized.

Contributor

aashish24 commented Jun 20, 2016

@danlipsa is this ready to be reviewed?

@danlipsa

This comment has been minimized.

Contributor

danlipsa commented Jun 20, 2016

@aashish24 No, we'll have to review/merge the VTK change first.
https://gitlab.kitware.com/vtk/vtk/merge_requests/1574

@doutriaux1

This comment has been minimized.

Member

doutriaux1 commented Jun 20, 2016

@danlipsa can you please create a branch in our vtk repo? that would make things easier to build, @aashish24 I though that was the reason we had our own fork of VTK so we don't need to wait.

@danlipsa

This comment has been minimized.

Contributor

danlipsa commented Jun 20, 2016

@doutriaux1 Just use the following remote
https://gitlab.kitware.com/danlipsa/vtk.git
and branch
remove-renderer-leak-followup

@doutriaux1

This comment has been minimized.

Member

doutriaux1 commented Jun 20, 2016

thank it is building now

@doutriaux1

This comment has been minimized.

Member

doutriaux1 commented Jun 21, 2016

@danlipsa when I run the follwoing:

import vcs
import cdms2
import os


f=cdms2.open(os.path.join(os.path.expanduser("~"),"clt.nc"))
data = f["clt"]
x=vcs.init()
for i in range(120):
    print i
    x.plot(data[i],"default","isofill",bg=1)
    x.png("file_%i.png" % i)
    x.clear()

i get:

Traceback (most recent call last):
  File "test_leak.py", line 11, in <module>
    x.plot(data[i],"default","isofill",bg=1)
  File "/usr/local/anaconda2/envs/LEAK/lib/python2.7/site-packages/vcs/Canvas.py", line 2398, in plot
    a = self.__plot(arglist, keyargs)
  File "/usr/local/anaconda2/envs/LEAK/lib/python2.7/site-packages/vcs/Canvas.py", line 3735, in __plot
    returned_kargs = self.backend.plot(*arglist, **keyargs)
  File "/usr/local/anaconda2/envs/LEAK/lib/python2.7/site-packages/vcs/VTKPlots.py", line 605, in plot
    vtk_backend_grid, vtk_backend_geo, **kargs))
  File "/usr/local/anaconda2/envs/LEAK/lib/python2.7/site-packages/vcs/vcsvtk/pipeline2d.py", line 312, in plot
    self._plotInternal()
  File "/usr/local/anaconda2/envs/LEAK/lib/python2.7/site-packages/vcs/vcsvtk/isofillpipeline.py", line 192, in _plotInternal
    self._gm, t, z, **kwargs))
  File "/usr/local/anaconda2/envs/LEAK/lib/python2.7/site-packages/vcs/VTKPlots.py", line 821, in renderTemplate
    displays = tmpl.plot(self.canvas, data, gm, bg=self.bg, **kargs)
  File "/usr/local/anaconda2/envs/LEAK/lib/python2.7/site-packages/vcs/template.py", line 1572, in plot
    **kargs)
  File "/usr/local/anaconda2/envs/LEAK/lib/python2.7/site-packages/vcs/template.py", line 1236, in drawTicks
    displays.append(x.line(ticks, bg=bg, **kargs))
  File "/usr/local/anaconda2/envs/LEAK/lib/python2.7/site-packages/vcs/Canvas.py", line 1835, in line
    return self.__plot(arglist, parms)
  File "/usr/local/anaconda2/envs/LEAK/lib/python2.7/site-packages/vcs/Canvas.py", line 3735, in __plot
    returned_kargs = self.backend.plot(*arglist, **keyargs)
  File "/usr/local/anaconda2/envs/LEAK/lib/python2.7/site-packages/vcs/VTKPlots.py", line 636, in plot
    cmap=self.canvas.colormap)
  File "/usr/local/anaconda2/envs/LEAK/lib/python2.7/site-packages/vcs/vcs2vtk.py", line 1709, in prepLine
    colors.InsertNextTypedTuple(vtk_color)
AttributeError: 'vtkCommonCorePython.vtkUnsignedCharArray' object has no attribute 'InsertNextTypedTuple'
@doutriaux1

This comment has been minimized.

Member

doutriaux1 commented Jun 21, 2016

my bad it picked up the wrong vtk

@danlipsa

This comment has been minimized.

Contributor

danlipsa commented Jun 21, 2016

@doutriaux1 @aashish24 I got the green light for my VTK fix so that will be in master shortly.

@danlipsa

This comment has been minimized.

Contributor

danlipsa commented Jun 21, 2016

@doutriaux1 @aashish24 The VTK change is in VTK master. How do we update uvcdat vtk? Do I bring in VTK master as a separate branch for uvcdat VTK?

@doutriaux1

This comment has been minimized.

Member

doutriaux1 commented Jun 21, 2016

@danlipsa you probably just want to do a PR of your changes into UV-CDAT. I'm not sure we want to bring all of VTK changes into UVCDAT/VTK (which will likely create a whole new set of baselines)

@doutriaux1

This comment has been minimized.

Member

doutriaux1 commented Jun 21, 2016

@danlipsa ok I tested it and it works great! Thanks. Let me know when your changes are merged into UV-CDAT/VTK I will merge this!

@danlipsa

This comment has been minimized.

Contributor

danlipsa commented Jun 21, 2016

@doutriaux1 At least the master from a few days ago, which I used as base for my change did not. I run the uvcdat tests with the old baselines and passed. So I think it is unlikely that the current master would change the baselines, using the conda set of libraries.

@danlipsa

This comment has been minimized.

Contributor

danlipsa commented Jun 21, 2016

@doutriaux1 Let's try it. If it does not create problems let's use it, rather than becoming more and more distanced from master VTK, and creating more problems when we do need to update to VTK master. We might get some free bug fixes (and hopefully no new bugs)

@danlipsa

This comment has been minimized.

Contributor

danlipsa commented Jun 21, 2016

@doutriaux1 OK, I'll try the master VTK and run the tests and see how it works.

@danlipsa danlipsa referenced this pull request Jun 22, 2016

Merged

Kitware master #15

@doutriaux1 doutriaux1 merged commit 921dd9b into master Jun 22, 2016

0 of 2 checks passed

continuous-integration/travis-ci/push The Travis CI build could not complete due to an error
Details
continuous-integration/travis-ci/pr The Travis CI build failed
Details

@doutriaux1 doutriaux1 deleted the vtk-update-leak branch Jun 22, 2016

@doutriaux1

This comment has been minimized.

Member

doutriaux1 commented Jun 22, 2016

@danlipsa I need to update VTK on conda repos now.

@danlipsa

This comment has been minimized.

Contributor

danlipsa commented Jun 22, 2016

@doutriaux1 Let me know when that is working so that I can try it on my machine.

@doutriaux1

This comment has been minimized.

Member

doutriaux1 commented Jun 22, 2016

@danlipsa master should be good to go now, please let me know.

@doutriaux1

This comment has been minimized.

Member

doutriaux1 commented Jun 22, 2016

on linux. Mac new VTK is still building

@danlipsa

This comment has been minimized.

Contributor

danlipsa commented Jun 22, 2016

Sounds good. I'll try it out on linux. Thanks.

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