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

conduit o2mrelation offsets has one less element than vtkCellArray offsets, which implies deep copy #1351

Open
danlipsa opened this issue Jan 13, 2025 · 2 comments
Labels

Comments

@danlipsa
Copy link

Both conduit o2mrelation offsets and vtkCellArray offsets store the index in the connectivity array where each cell starts.
However, vtkCellArray offsets stores numCells + 1 values while o2mrelation offsets stores numCells values. The last value in vtkCellArray offsets is always the length of the connectivity array. It is used as a sentinel, eliminating an additional test for the end of the array.

VTKm uses the same convention as VTK.

We cannot simply pass the pointer to the offsets array coming from conduit o2mrelation to be able to use it in vtkCellarray offsets. We have to copy that array and add an element to it.

@danlipsa
Copy link
Author

@cyrush Do you have any insights on this? Thanks!

@cyrush
Copy link
Member

cyrush commented Jan 13, 2025

@danlipsa That is correct, o2m includes an entry per mapping, it doesn't have an extra entry at the end.

The length is handled by the Conduit leaf data type description of the array (it's not encoded in the array data itself). o2ms are used for general relationships not just connectivity arrays.

Despite what the docs currently say - sizes, offsets are both required.

We could think about a special case to allow the lenght of sizes and offsets to differ for this case - but folks would need to detect and discriminate against those cases. Since we use this for more than cell connectivity, I worry that logic to match vtk + vtk-m will confuse things elsewhere.

So this limits what we can zero-copy to VTK/VTK-m. Definitely not asking VTK/VTK-m to change, I know that would be disruptive.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants