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

Vtk update leak #2030

Merged
merged 2 commits into from
Jun 22, 2016
Merged

Vtk update leak #2030

merged 2 commits into from
Jun 22, 2016

Conversation

danlipsa
Copy link
Contributor

No description provided.

@aashish24
Copy link
Contributor

@danlipsa is this ready to be reviewed?

@danlipsa
Copy link
Contributor Author

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

@doutriaux1
Copy link
Contributor

@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
Copy link
Contributor Author

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

@doutriaux1
Copy link
Contributor

thank it is building now

@doutriaux1
Copy link
Contributor

@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
Copy link
Contributor

my bad it picked up the wrong vtk

@danlipsa
Copy link
Contributor Author

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

@danlipsa
Copy link
Contributor Author

@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
Copy link
Contributor

@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
Copy link
Contributor

@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
Copy link
Contributor Author

@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
Copy link
Contributor Author

@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
Copy link
Contributor Author

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

@danlipsa danlipsa mentioned this pull request Jun 22, 2016
@doutriaux1 doutriaux1 merged commit 921dd9b into master Jun 22, 2016
@doutriaux1 doutriaux1 deleted the vtk-update-leak branch June 22, 2016 14:24
@doutriaux1
Copy link
Contributor

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

@danlipsa
Copy link
Contributor Author

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

@doutriaux1
Copy link
Contributor

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

@doutriaux1
Copy link
Contributor

on linux. Mac new VTK is still building

@danlipsa
Copy link
Contributor Author

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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants