When asked to grow current allocation, vtkAbstractArray::Resize will reallocate space for the number of tuples using vtkGenericDataArray::ReallocateTuples and leave MaxId untouched. On the other hand, when asked to shrink, the MaxId will change. This cause confusion for C++ developers used to STL.
Let’s see how STL vector’s reserve and resize work.
vector<int> v;
// reserve will change capacity but not the size.
v.reserve(20);
assert(v.capacity() == 20) // ✅
assert(v.size() == 20) // ❌
// resize will change capacity and the size, cuz re*size*
v.resize(20);
assert(v.capacity() == 20) // ✅
assert(v.size() == 20) // ✅
Now, let’s contrast that with how VTK’s analogues for vector::reserve : ReallocateTuples and vector::resize : Resize work. See the last bit.
vtkNew<vtkTypeInt32Array> a;
// reallocate currently does not change MaxId, but changes Size.
a->ReallocateTuples(20)
assert(a->GetSize() == 20) // ✅
assert(a->GetNumberOfTuples() == 20) // ❌
// resize currently does not change MaxId, but changes Size.
a->Resize(20);
assert(a->GetSize() == 20) // ✅
assert(a->GetNumberOfTuples() == 20) // ❌
Proposed behavior
vtkAbstractArray::Resize will change MaxId so that GetNumberOfTuples and SetTuple will work as expected. This could break existing implementation of InsertNextTuple and InsertTuple, but we can fix it without checking for MaxId.
I’ve heard an additional proposal from @spyridon97 to rename Resize to ReallocateTuples. Thoughts and opinions welcome!
First proposal sounds good. However, let’s try not to break the CRTP mechanism in use everywhere.
Deprecate ReallocateTuples(numTuples) from vtkGenericDataArray, since Reallocate exists now.
Why not leave that as it is? vtkAbstractArray::Reallocate does nothing really, it’s a pure virtual. vtkGenericDataArray::Reallocate can do same thing as vtkGenericDataArray::Resize without changing MaxId
vtkGenericDataArray::ReallocateTuples offered a way for the derived classes to implement reallocation using CRTP. ReallocateTuples is one of the 8 concept methods defined in vtkGenericDataArray.html If we removed it, how do they implement reallocation now? For example, the implementation of vtkmDataArray allocates space in the device memory using vtk-m.
Maybe rename “Resize()” to “Reserve()”, instead of “Reallocate()”?
The protected method “ReallocateTuples()” does not need to change.
We already have “SetNumberOfTuples()”, so we don’t need to add a “Resize()” method that does exactly the same thing.
I’m fine with deprecating “Resize()”. I suspect that when it is deprecated, people will discover that some code used it incorrectly.
In the future, we might also want to deprecate “InsertTuple()”, too, because it is so different from stl behavior. “InsertNextTuple()” is still good, of course.
Edit: “ReserveTuples()” instead of just “Reserve()”
Discussing the logic behind the VTK API is valuable and describing the conclusion in the documentation is useful.
However, changing the API just to make it nicer or more consistent with STL should be very low on anyone’s priority list. The result would be a marginal improvement (slightly less chance of someone making error by reading a method’s name but not its documentation) and it may have high cost if many VTK-based projects and examples need to be changed.
VTK’s API has been working quite well over the past few decades. If we want to improve it for the next few decades then we should look beyond immediate ideas and needs. For example, instead of consistency with STL (which is kind of thing of the past and present), we should aim for helping AI code generator tools to create better code for users of VTK. Changing the API may invalidate many examples circulating on the internet that AI has been trained on, so it may have an immediate detrimental effect. Adding clarifying comments to the API documentation and improving quality and amount of examples may help both humans and AI more effectively. It may be too early to know where things go in the next few years and what exactly will be helpful for AI algorithms, so in general I would just generally try to avoid API churn until we know better.
There are also so many more important things to spend time on, such as the 248 open merge requests, 953 issues, and various strategically important improvements (VTK remote modules, Boolean mesh operations, …). So, I hope that these proposals for renaming a method or changing behavior of fundamental VTK classes remain just ideas for now.
