]> https://gitweb.dealii.org/ - dealii.git/commitdiff
Fix, and deprecate, a PETSc parallel vector copy constructor.
authorDavid Wells <wellsd2@rpi.edu>
Sat, 11 Feb 2017 22:29:44 +0000 (17:29 -0500)
committerDavid Wells <wellsd2@rpi.edu>
Sat, 11 Feb 2017 23:08:56 +0000 (18:08 -0500)
The former implementation of this copy constructor contained a subtle bug: since
VectorBase::operator= is implicitly defined, calling it simply copies (byte for
byte) the members of a VectorBase. This is erroneous because now two different
destructors will call VecDestroy on the underlying Vec.

The fix is simple: (nearly) preserve the current behavior by just deep copying
the vector.

Since one should never have an explicit instance of VectorBase (and there is no
reasonable way to copy a parallel vector and then enforce a possibly different
local_size) this constructor I also marked this constructor for deprecation: one
should use one of the concrete classes (Vector or MPI::Vector) in applications.

include/deal.II/lac/petsc_parallel_vector.h
source/lac/petsc_parallel_vector.cc

index 2713b630728b79f254c60ac2038b7448007d81ba..e61ae3d9c737973f4afd7dc1d6de697f021d934d 100644 (file)
@@ -227,10 +227,13 @@ namespace PETScWrappers
        *
        * @arg communicator denotes the MPI communicator over which the
        * different parts of the vector shall communicate
+       *
+       * @deprecated The use of objects that are explicitly of type VectorBase
+       * is deprecated: use PETScWrappers::MPI::Vector instead.
        */
       explicit Vector (const MPI_Comm     &communicator,
                        const VectorBase   &v,
-                       const size_type     local_size);
+                       const size_type     local_size) DEAL_II_DEPRECATED;
 
       /**
        * Construct a new parallel ghosted PETSc vector from an IndexSet.
index 6073e3dc556756ba1b5553130d7adfe46643cb3d..bd519019c8c367c66d9fe7436a0e7448f0361acb 100644 (file)
@@ -56,11 +56,19 @@ namespace PETScWrappers
                     const VectorBase  &v,
                     const size_type   local_size)
       :
+      VectorBase (v),
       communicator (communicator)
     {
-      Vector::create_vector (v.size(), local_size);
-
-      VectorBase::operator = (v);
+      // In the past (before it was deprecated) this constructor did a
+      // byte-for-byte copy of v. This choice resulted in two problems:
+      // 1. The created vector will have the same size as v, not local size.
+      // 2. Since both the created vector and v maintain ownership of the same
+      // PETSc Vec, both will try to destroy it: this does not make sense.
+      //
+      // For the sake of backwards compatibility, preserve the behavior of the
+      // copy, but correct the ownership bug. Note that in both this (and the
+      // original) implementation local_size is ultimately unused.
+      (void)local_size;
     }
 
 

In the beginning the Universe was created. This has made a lot of people very angry and has been widely regarded as a bad move.

Douglas Adams


Typeset in Trocchi and Trocchi Bold Sans Serif.