Let dlx.la.Vector manage the scope of the petsc4py wrapper (#1) #3092
Conversation
This PR allows for a `dlx.la.Vector` (rather than a `dlx.fem.Function`) to manage the memory and the scope of the `petsc4py.PETSc.vec` object that wraps the underlying data. The implementation of the property `dlx.fem.Function.vector` was also modified to delegate the creation of the wrapper to the underlying `dlx.la.Vector`. All changes are backward compatible.
| @@ -485,7 +480,7 @@ def copy(self) -> Function: | |||
| @property | |||
| def x(self) -> la.Vector: | |||
| """Vector holding the degrees-of-freedom.""" | |||
| return la.Vector(self._cpp_object.x) # type: ignore | |||
| return self._x # type: ignore | |||
|
|
|||
| @property | |||
| def vector(self): | |||
There was a problem hiding this comment.
We should probably deprecate this method.
There was a problem hiding this comment.
Yes, we can deprecate the method. However, I would like to get consensus on this, since it will require to update all the python examples using Function.vector to use Function.x.petsc_vec
There was a problem hiding this comment.
I think we should put in a deprecation-warning, and remove it with the release of 0.9.x
There was a problem hiding this comment.
Sounds good. I'll do that and replace all occurrences of u.vector to u.x.petsc_vec.
There was a problem hiding this comment.
@garth-wells @jorgensd : This has been addressed. All occurrences of u.vector were replaced with u.x.petsc_vec and a deprecation warning was added to dlx.fem.Function.vector.
python/dolfinx/la.py
Outdated
| @@ -245,6 +250,18 @@ def array(self) -> np.ndarray: | |||
| """Local representation of the vector.""" | |||
| return self._cpp_object.array | |||
|
|
|||
| @property | |||
| def vector(self): | |||
There was a problem hiding this comment.
I think we need a more descriptive name. Maybe petsc_vec?
There was a problem hiding this comment.
I also prefer petsc_vec. I originally used vector since this was the method used in the fem.Function class.
I will make the update.
There was a problem hiding this comment.
@garth-wells : This has been renamed as recommended.
Co-authored-by: Garth N. Wells <gnw20@cam.ac.uk>
Remove # type: ignore Co-authored-by: Garth N. Wells <gnw20@cam.ac.uk>
|
@garth-wells @jhale @jorgensd : Requested changes have been addressed. We are about to opensource Thank you |
Fix spelling in warnings message.
Specify that it is a deprecation warning
- Also add some additional `create_connectivity(tdim, tdim)` calls prior to midpoint computations (consequence of FEniCS/dolfinx#3209) - Switch to spdlog and move some logging to debug mode (FEniCS/dolfinx#3216) - Support nanobind 2.0.0 (FEniCS/dolfinx#3105) - Replace `dolfinx.Function.vector` with `dolfinx.Function.x.petsc_vec` (FEniCS/dolfinx#3092)
This PR allows for a
dlx.la.Vector(rather than adlx.fem.Function) to manage the memory and the scope of thepetsc4py.PETSc.vecobject that wraps the underlying data.The implementation of the property
dlx.fem.Function.vectorwas also modified to delegate the creation of the wrapper to the underlyingdlx.la.Vector.All changes are backward compatible.
See also Issue #3061
@jorgensd
@garth-wells
@jhale