-
-
Notifications
You must be signed in to change notification settings - Fork 217
Fix vector block data for single function space case #3924
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
Conversation
@@ -123,7 +123,10 @@ def create_vector( | |||
index_map, bs = maps[0] | |||
ghosts = index_map.ghosts.astype(PETSc.IntType) # type: ignore[attr-defined] | |||
size = (index_map.size_local * bs, index_map.size_global * bs) | |||
return PETSc.Vec().createGhost(ghosts, size=size, bsize=bs, comm=index_map.comm) # type: ignore | |||
b = PETSc.Vec().createGhost(ghosts, size=size, bsize=bs, comm=index_map.comm) # type: ignore | |||
if kind == PETSc.Vec.Type.MPI: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if kind == None
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the previous behaviour that would have been equivalent to the call create_vector(L)
which did not attach block data, therefore we don't want to attach data.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If creating vector for list of a single space, i.e. create_vector([V])
is almost the same as creating a vector on a single space create_vector(V)
, except that the former attaches block data, then why don't we remove the whole concept of single space create_vector(V)
and always require a list of spaces, be it of length one. That would simplify meaning of kind
, in my opinion, so either mpi or nest. @garth-wells ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know I added the 'block data' concept but I can't recall the details ;). Does it introduce any memory overhead for the single space case?
In general, I'm in favour of as few special cases as possible, and has few interface types and possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about merging this, which recovers behaviour as before (and fixes down stream usage) and having a look into removing the difference in another PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree.
With the new interface, the
create_vector([V], kind="mpi")
case no longer holds block data. This adds the block data, allowing to treat a single function space vector as a block vector when passingtype="mpi"
. Caught downstream.