-
-
Notifications
You must be signed in to change notification settings - Fork 194
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
Use std::optional
to reduce code duplication in boundary condition setting
#3434
Conversation
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 for doing this! std::optional
is great for us, we should carefully roll it out everywhere.
std::span<T> b(_b.mutable_array()); | ||
std::ranges::fill(b, 0.0); | ||
std::span b(_b.mutable_array()); | ||
std::ranges::fill(b, 0); |
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.
Tangential, but do we have a set style on this? Personally I prefer using floating point literals even if a cast from an integer literal is possible. We are also using T(1)
below.
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 our house (0.0
) style pre-dates supporting different types. I prefer
std::ranges::fill(b, 0);
to
std::ranges::fill(b, T(0));
because it's less syntax. I think
std::ranges::fill(b, 0.0);
is possibly misleading since T
is not necessarily double
.
@@ -188,7 +188,7 @@ def action_A(x, y): | |||
y.scatter_reverse(la.InsertMode.add) | |||
|
|||
# Set BC dofs to zero | |||
fem.set_bc(y.array, [bc], scale=0.0) | |||
bc.set(y.array, alpha=0.0) |
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 is a nice way of homogenising the bc, which is super convenient for optimization.
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.
We already have a function that can do this more 'directly'
dolfinx/cpp/dolfinx/fem/DirichletBC.h
Line 557 in bc88ca0
/// @todo Review this function - it is almost identical to the |
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've added an optimisation in DirichletBC::set
for when alpha == 0
, and remove the function that was largely a duplicate.
See #3413.
Also: