Skip to content

Conversation

@jorgensd
Copy link
Member

@jorgensd jorgensd commented Sep 26, 2025

For blocked function spaces, locate_dofs_topological and DirichletBC assumes blocked indices.
However, the private functions num_owned assumes that the dofs are unrolled, which makes the number of owned indices returned by DirichletBC::dof_indices wrong.

@chrisrichardson
Copy link
Contributor

chrisrichardson commented Sep 26, 2025

Looks like bs is being applied twice at the moment (is that right?), once in num_owned and then again in constructor

@jorgensd
Copy link
Member Author

Looks like bs is being applied twice at the moment (is that right?), once in num_owned and then again in constructor

It is being applied twice, but it also assumes that the initial dofs are unrolled, meaning that it captures too many degrees of freedom as owned. So there are two different things that can cause issues at the moment.

jorgensd added a commit that referenced this pull request Sep 26, 2025
@michalhabera
Copy link
Contributor

Good catch. But what is _owned_indices0 used for apart from returning in dof_indices()? Seems to me a bit weird to return it. With your fix it will return unrolled dofs and number of not unrolled owned dofs, so a bit inconsistent.

@jorgensd
Copy link
Member Author

Good catch. But what is _owned_indices0 used for apart from returning in dof_indices()? Seems to me a bit weird to return it. With your fix it will return unrolled dofs and number of not unrolled owned dofs, so a bit inconsistent.

It still returns the number of unrolled dofs, as it is multiplied by bs.
I caught this in: #3938 where I require it to set boundary conditions on the Python side of things (to the local PETSc vector)

@michalhabera
Copy link
Contributor

Good catch. But what is _owned_indices0 used for apart from returning in dof_indices()? Seems to me a bit weird to return it. With your fix it will return unrolled dofs and number of not unrolled owned dofs, so a bit inconsistent.

It still returns the number of unrolled dofs, as it is multiplied by bs. I caught this in: #3938 where I require it to set boundary conditions on the Python side of things (to the local PETSc vector)

Ah right, missed the multiplication later somewhere in constructor... This is good to merge as a bugfix, but unrelated to this PR, returning an array and number of owned elements in the array is not a common dolfinx pattern, isn't it? We've always relied on user figuring this out.

@jorgensd jorgensd added this pull request to the merge queue Sep 26, 2025
Merged via the queue into main with commit c698af0 Sep 26, 2025
20 checks passed
@jorgensd jorgensd deleted the dokken/blocked_bcs_ownership branch September 26, 2025 16:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants