Update input refactor #1450
Update input refactor #1450
Conversation
|
Looks like there's an issue with the testdata sync. I'll fix this now... |
Otherwise it will reset WIP branches to use master.
Adding this logic to a filter will allow us to setup the pipelines and simply re-execute them, rather than rebuilding them for each frame of an animation.
Until the reference cycles that prevent the canvases from being cleaned up when going out of scope are cleaned up, exiting without closing will trigger a segfault.
Modified some pipelines to remove SetInputData calls to ensure than an actual visualization pipeline exists. It is no longer necessary to rebuild the pipeline between animation steps, just update the inputs and reexecute it. This removes a half-finished implementation of update_input for vector animation. Since it wasn't working / tested, I'll push a separate branch that contains the ported implementation and adds a test, though there is still work to be done to get it working.
alliepiper
force-pushed
the
update_input_refactor
branch
from
fab23e8 to
b4199e3
Compare
|
Should be fixed. Works here, we'll see if the buildbot picks it up. |
|
That did it -- ready for review/testing. |
|
@dlonie what was the issue with atexit and its callback? |
|
Registering a bound method with More concretely, |
| @@ -122,8 +122,6 @@ def __init__(self,vcs_self): | |||
| self.renderers = [] | |||
| self.last_size = None | |||
| self.modified_listener = None | |||
| import atexit | |||
| atexit.register(self.close) | |||
There was a problem hiding this comment.
@dlonie I'm about to test now. But did you check that pngs are now cleared? The reason this was put in is that the del function was never reached hence leaving tons of temporary png file around.
There was a problem hiding this comment.
maybe we can unregister it in the close function? Anyway I will double check the pngs are removed properly. Maybe I will simply use tempfile module instead in order to make sure the pngs are removed.
There was a problem hiding this comment.
As long as close is called explicitly, it should behave just as before. The reason __del__ wasn't getting called is likely due to reference leaks -- if there are lingering references due to cycles or atexit registrations, __del__ will never be called since the garbage collector will never be notified to clean the objects up.
+1 for using platform temporary files instead of the home directory.
|
@dlonie on my ubuntu all animation tests seg fault now. see: https://open.cdash.org/viewTest.php?onlyfailed&buildid=3901832 |
|
on my mac all test fail... but I think it's an issue with ctest not picking up the right python. See: |
|
Which version of Ubuntu? Can you rerun the mac tests in a new shell session? Looks like that should fix that. |
|
python trace back: |
|
Yep -- that's the stack from the crash when a We might just have to hold off on merging this until the reference leaks are fixed. I'd gotten this working on Arch Linux, but it looks like some of the leaks are platform specific. |
|
same seg fault on mac, plus a few more, see: https://open.cdash.org/viewTest.php?onlyfailed&buildid=3901852 |
|
@dlonie ubunutu 15. Also pointed to newer mac test with correct python picked up. |
|
@doutriaux1 could you please look into these seg fautls as they seem to be related to leaks.. |
|
I'll split this branch up into two branches: one with the bug fixes and another with the last bit of the refactor. We can get the bugfixes in and merge the refactor branch once the memory leaks are cleaned up so that we can start using the |
|
@UV-CDAT/developers I am closing this as we have most of the components in master now. |
@aashish24 @doutriaux1 @sankhesh @chaosphere2112
Done with the
update_inputrefactor and ported the VTK pipeline-related stuff from VTKPlots.update_input into the pipeline classes.There was a unfinished/nonworking/untested bit of
update_inputthat was for vector plots that gets removed by this PR. I'll push another branch (no PR) shortly and file a bug for this.I ended up leaving the map returned from pipeline.plot(...) in and just fixed the leaks that were causing the segfaults. Now Canvas.close() must be called in user scripts to avoid the crashes. These can be further refined in the future as needed. See af6eafd and dd81ee3. Once the rest of the leaks that are preventing canvases from cleaning up when going out of scope are cleaned up, this caveat can go away.
vcs2vtk.doWrapis now wrapped in a VTK filter so that it can be called as part of the VTK pipeline execution. See 457c5fd to see how this can be done for other parts of the code that may need it down the road.Goes with CDAT/uvcdat-testdata#58, as this fixed some bugs and many of the vcs baselines were actually incorrect.