pyvista / pyvista-xarray

xarray DataArray accessors for PyVista
Apache License 2.0
98 stars 6 forks source link

Memory sharing issues after PyVista 0.37 for RectilinearGrid #44

Open banesullivan opened 1 year ago

banesullivan commented 1 year ago

See 0e64e7d

https://github.com/pyvista/pyvista/pull/3179 broke the test_convert_vtr test when it validates that the X and Y coordinates share memory but not Z. This is super odd and I cannot figure out what is going wrong as the changes in https://github.com/pyvista/pyvista/pull/3179 do not seemingly affect this. So for some reason X and Y are DEEP copied now while Z is still SHALLOW copied.

@MatthewFlamm, I hate to throw you in the deep end but I'm wondering if you might have any insights come to mind as to why that pull request would have broken this test.

banesullivan commented 1 year ago

the reality is that a lot of the code here in pyvista-xarray for ensuring shared arrays is very fragile and needs major improvement

MatthewFlamm commented 1 year ago

This is weird. This test does not pass in a vtk object at all to Rectilinear Grid as far as I can tell. One line reads in a file, the other instantiates a blank object and then manually constructs it via attributes here https://github.com/pyvista/pyvista-xarray/blob/0e64e7df029eef1f9090a5a017ac05c7244ec23e/pvxarray/rectilinear.py#L24-L29

Other weird thing is that the prior behavior for Rectilinear grid was to always deep copy vtk objects.

MatthewFlamm commented 1 year ago

Have you bisected it to this exact commit in PyVista? Or just by version?

banesullivan commented 1 year ago

I bisected, and it was the commit for https://github.com/pyvista/pyvista/pull/3179