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
Add MatrixFreeTools::ElementBirthAndDeathMatrixFree #14121
Conversation
@sebproell and I have been working on resolving issue #12102. We propose not to integrate the notion of FYI @Rombur you have open the original issue. Would this approach work for you? |
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.
Thanks @peterrum!
include/deal.II/matrix_free/tools.h
Outdated
* FENothing assigned to them. In the following we call such cells | ||
* invalid. All other cells are valid. In contrast to MatrixFree, | ||
* this class skips invalid cells and faces between valid and | ||
* invalid cells are treated as boundary faces. |
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.
Maybe mention how to figure out if a face is an actual boundary face or a valid-invalid boundary face inside the user-supplied boundary operation?
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 would use this function: #14122 :)
const unsigned int type = | ||
static_cast<unsigned int>(category.first == fe_index_valid) + | ||
static_cast<unsigned int>(category.second == fe_index_valid); | ||
|
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.
Would Assert(type != 0, ExcInternalError())
be useful here?
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.
That is invalid face and nothing is done and the face is skipped. I have added a comment.
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.
This looks good to me. My main concern is that we introduce the concept of boundary inside the domain which we have always forbidden.
include/deal.II/matrix_free/tools.h
Outdated
|
||
/** | ||
* Loop over all valid cells and faces. Faces between valid and | ||
* invalid cells are treated as boundary faces. |
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 agree that it's what most people want but it is very different than what we do everywhere else. That's fine with me but that's something we need to think about. Since the face is treated as a boundary, which boundary ID is used?
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 agree that it's what most people want but it is very different than what we do everywhere else. That's fine with me but that's something we need to think about.
That is why it is not part of MatrixFree
actual. This is helper class tailored for such purposes.
Since the face is treated as a boundary, which boundary ID is used?
I would say: numbers::internal_face_boundary_id
. See #14122
0001d73
to
06ceda0
Compare
/rebuild |
is a wrapper around MatrixFree, which helps users to deal with | ||
DoFHandlers, where there are cells without degrees of freedom, | ||
i.e., with FENothing assigned to them. The class helps to implement | ||
"element birth and death technique". |
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.
Is this a term used anywhere else on the literature? If not, then I would suggest to with something like activation/ deactivation instead.
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.
You mean ElementActivationAndDeactivationMatrixFree
?
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.
Yes, something like that sounds better to me. Would that be acceptable?
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.
Renamed!
1ca3b8e
to
9c3e1e3
Compare
if (fe_collection[i].n_dofs_per_cell() > 0) | ||
valid_fe_indices.insert(i); | ||
|
||
AssertDimension(valid_fe_indices.size(), 1); |
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.
Can we allow more than one valid_fe_indices
? Or create an invalid_fe_index
and check for it later?
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.
Can we allow more than one valid_fe_indices?
Yes, we can. Also, I guess what you need in your case is to not loop over faces neighboring FE_Nothing
.
Before making the modifications I would like to get to a status where we agree that the class is useful, we found a name, and maybe merged the first version so that we can improve it from there.
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.
Also, I guess what you need in your case is to not loop over faces neighboring FE_Nothing.
Yes, this should only be optional from my perspective. For me this class can definitely be useful, currently I skip FENothing cells (or cell batches) manually.
9c3e1e3
to
f51943b
Compare
f51943b
to
56856ae
Compare
With #14122 being merged, I have rebased this PR! Ready from my side. |
* deactivated. All other cells are activated. In contrast to MatrixFree, | ||
* this class skips deactivated cells and faces between activated and | ||
* deactivated cells are treated as boundary faces. |
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 think it would be important to say that this class considers only the first FE
in an hp::FECollection
that has non-zero number of unknowns (if I read the code correctly).
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 have added a comment. To lift this restriction is the next step; see #14121 (comment).
56856ae
to
1500599
Compare
@kronbichler I have incorporated your suggestion! Thanks. |
Add MatrixFreeTools::ElementBirthAndDeathMatrixFree
references #12102
depends on #14122