[petsc-dev] vec/vec/examples/tests/ex37.c "Nest vector functionality. has bugs

[Barry Smith]

On Feb 20, 2011, at 2:07 PM, Jed Brown wrote:

> On Sun, Feb 20, 2011 at 20:52, Barry Smith <bsmith at mcs.anl.gov> wrote:
> 
>  ierr = VecDestroy(a);CHKERRQ(ierr);   ierr = VecDestroy(b);CHKERRQ(ierr);
> 
>  ierr = VecAssemblyBegin(X);CHKERRQ(ierr);
>  ierr = VecAssemblyEnd(X);CHKERRQ(ierr);
> 
>  ierr = VecMax( b, &index, &val );CHKERRQ(ierr);
>  PetscPrintf( PETSC_COMM_WORLD, "(max-b) = %f : index = %d \n", val, index );
> 
>  b is destroyed and then used later.
> 
> Well, sort of. The Vec is not actually destroyed because the VecNest X retains a reference. VecDestroy is called to relinquish ownership, not to actually free the memory. Although this is perhaps not very pretty, neither is calling PetscObjectDereference((PetscObject)b) for the same purpose. I think that waiting until the end of the function body to call VecDestroy is harder to reason about: when you read the function from the top down, ownership becomes ambiguous and you have to scan to the end of the function body to find out who keeps ownership.
> 
> In this example, the max could easily be computed before creating X, but I guess we need a style guideline for other cases where we want to relinquish ownership, but still be able to inspect. The verbose choice is to VecDestroy and then immediately call VecNestGetSubVec (a low-level function that I would prefer people not need to call very often) or VecGetSubVector (the generic function, but you need index sets for that, not just PETSC_NULL) to get a non-ownership reference to the same thing you passed in.
> 
> Or still use VecDestroy, but always include a comment about it just relinquishing ownership. I realize that usage conflicts with your plan to make XXDestroy() or XXDestroy_() or something zero the pointer (which by no means prevents stale references, but takes care of some of the obvious cases).


  I originally planned that PetscObjectDereference() was the thing to use in this case. From its manual page

PetscObjectDereference - Indicates to any PetscObject that it is being
   referenced by one less PetscObject. This decreases the reference
   count for that object by one.

but I now realize the name is not great. It implies that this particular reference is being removed. Could change it to PetscObjectDecreaseReference() and have it 
zero the pointer only if it got down to zero. 

  I really hate code that does XXXDestroy(x) then XXXXSomething(x); it is fundamentally confusing

  Barry