-
Notifications
You must be signed in to change notification settings - Fork 182
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
Pass fluxes to elliptic source computers #2553
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -43,34 +43,44 @@ struct FirstOrderFluxesImpl<Dim, tmpl::list<PrimalFields...>, | |
} | ||
}; | ||
|
||
template <typename PrimalFields, typename AuxiliaryFields, | ||
template <size_t Dim, typename PrimalFields, typename AuxiliaryFields, | ||
typename SourcesComputer> | ||
struct FirstOrderSourcesImpl; | ||
|
||
template <typename... PrimalFields, typename... AuxiliaryFields, | ||
template <size_t Dim, typename... PrimalFields, typename... AuxiliaryFields, | ||
typename SourcesComputer> | ||
struct FirstOrderSourcesImpl<tmpl::list<PrimalFields...>, | ||
struct FirstOrderSourcesImpl<Dim, tmpl::list<PrimalFields...>, | ||
tmpl::list<AuxiliaryFields...>, SourcesComputer> { | ||
template <typename VarsTags, typename... SourcesArgs> | ||
static constexpr void apply( | ||
const gsl::not_null< | ||
Variables<db::wrap_tags_in<::Tags::Source, VarsTags>>*> | ||
sources, | ||
const Variables<VarsTags>& vars, | ||
const Variables<db::wrap_tags_in< | ||
::Tags::Flux, VarsTags, tmpl::size_t<Dim>, Frame::Inertial>>& fluxes, | ||
const SourcesArgs&... sources_args) noexcept { | ||
// Compute sources for primal fields | ||
SourcesComputer::apply( | ||
make_not_null(&get<::Tags::Source<PrimalFields>>(*sources))..., | ||
sources_args..., get<PrimalFields>(vars)...); | ||
// Compute sources for auxiliary fields. They are just the auxiliary field | ||
// values. | ||
// Set auxiliary field sources to the auxiliary field values to begin with. | ||
// This is the standard choice, since the auxiliary equations define the | ||
// auxiliary variables. Source computers can adjust or add to these sources. | ||
tmpl::for_each<tmpl::list<AuxiliaryFields...>>( | ||
[&sources, &vars ](const auto auxiliary_field_tag_v) noexcept { | ||
[&sources, &vars](const auto auxiliary_field_tag_v) noexcept { | ||
using auxiliary_field_tag = | ||
tmpl::type_from<decltype(auxiliary_field_tag_v)>; | ||
get<::Tags::Source<auxiliary_field_tag>>(*sources) = | ||
get<auxiliary_field_tag>(vars); | ||
}); | ||
// Call into the sources computer to set primal field sources and possibly | ||
// adjust auxiliary field sources. The sources depend on the primal and the | ||
// auxiliary variables. However, we pass the volume fluxes instead of the | ||
// auxiliary variables to the source computer as an optimization so they | ||
// don't have to be re-computed. | ||
SourcesComputer::apply( | ||
make_not_null(&get<::Tags::Source<PrimalFields>>(*sources))..., | ||
make_not_null(&get<::Tags::Source<AuxiliaryFields>>(*sources))..., | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You keep saying "instead of the auxiliary fields" but here you pass the aux fields. Please correct this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No this passes the sources for the auxiliary fields so terms can be added to them, such as Christoffel symbol terms (see this comment #2553 (comment)). Does that make sense? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Okay, that makes sense. I still don't understand why you say "instead of" because the code didn't previously pass the aux vars. Why wouldn't you need the aux vars (I realize they weren't passed before)? Is that because technically they are just (roughly) the derivatives of the primal fields? If so, I'm a bit confused that stability and well-posedness aren't adversely affected by never using the aux variables, though explaining that is beyond the scope of a GitHub comment, I think ;) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Previously I didn't pass In the comments I say "instead of" because the sources are a function of the variables (primal and auxiliary), but instead of the aux vars I pass the primal fluxes as an optimization, as we discussed before. Does this clear things up? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, but I think this lengthy discussion suggests that you should either remove the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Adjusted the commit message and the code comment in the fixup commit, please take a look. |
||
sources_args..., get<PrimalFields>(vars)..., | ||
get<::Tags::Flux<PrimalFields, tmpl::size_t<Dim>, Frame::Inertial>>( | ||
fluxes)...); | ||
} | ||
}; | ||
|
||
|
@@ -118,30 +128,41 @@ auto first_order_fluxes(const Variables<VarsTags>& vars, | |
* \brief Compute the sources \f$S(u)\f$ for the first-order formulation of | ||
* elliptic systems. | ||
* | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. add a discussion as to why the sources terms are getting the fluxes |
||
* This function takes the `fluxes` as an argument in addition to the variables | ||
* as an optimization. The fluxes will generally be computed before the sources | ||
* anyway, so we pass them to the source computers to avoid having to re-compute | ||
* them for source-terms that have the same form as the fluxes. | ||
* | ||
* \see `elliptic::first_order_operator` | ||
*/ | ||
template <typename PrimalFields, typename AuxiliaryFields, | ||
template <size_t Dim, typename PrimalFields, typename AuxiliaryFields, | ||
typename SourcesComputer, typename VarsTags, typename... SourcesArgs> | ||
void first_order_sources( | ||
gsl::not_null<Variables<db::wrap_tags_in<::Tags::Source, VarsTags>>*> | ||
sources, | ||
const Variables<VarsTags>& vars, | ||
const Variables<db::wrap_tags_in<::Tags::Flux, VarsTags, tmpl::size_t<Dim>, | ||
Frame::Inertial>>& fluxes, | ||
const SourcesArgs&... sources_args) noexcept { | ||
first_order_operator_detail::FirstOrderSourcesImpl< | ||
PrimalFields, AuxiliaryFields, SourcesComputer>::apply(std::move(sources), | ||
vars, | ||
sources_args...); | ||
Dim, PrimalFields, AuxiliaryFields, | ||
SourcesComputer>::apply(std::move(sources), vars, fluxes, | ||
sources_args...); | ||
} | ||
|
||
template <typename PrimalFields, typename AuxiliaryFields, | ||
template <size_t Dim, typename PrimalFields, typename AuxiliaryFields, | ||
typename SourcesComputer, typename VarsTags, typename... SourcesArgs> | ||
auto first_order_sources(const Variables<VarsTags>& vars, | ||
const SourcesArgs&... sources_args) noexcept { | ||
auto first_order_sources( | ||
const Variables<VarsTags>& vars, | ||
const Variables<db::wrap_tags_in<::Tags::Flux, VarsTags, tmpl::size_t<Dim>, | ||
Frame::Inertial>>& fluxes, | ||
const SourcesArgs&... sources_args) noexcept { | ||
Variables<db::wrap_tags_in<::Tags::Source, VarsTags>> sources{ | ||
vars.number_of_grid_points()}; | ||
first_order_operator_detail::FirstOrderSourcesImpl< | ||
PrimalFields, AuxiliaryFields, | ||
SourcesComputer>::apply(make_not_null(&sources), vars, sources_args...); | ||
Dim, PrimalFields, AuxiliaryFields, | ||
SourcesComputer>::apply(make_not_null(&sources), vars, fluxes, | ||
sources_args...); | ||
return sources; | ||
} | ||
// @} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -94,7 +94,9 @@ struct Sources { | |
using argument_tags = tmpl::list<>; | ||
static void apply( | ||
const gsl::not_null<tnsr::I<DataVector, Dim>*> source_for_displacement, | ||
const tnsr::I<DataVector, Dim>& /*displacement*/) noexcept { | ||
const gsl::not_null<tnsr::ii<DataVector, Dim>*> /*source_for_strain*/, | ||
const tnsr::I<DataVector, Dim>& /*displacement*/, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. aren't the new arguments fluxes? Maybe make that clear in the name? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Okay, I can't speak to what would be most intuitive for people used to the elasticity literature, so I'll have to take your word for it :) |
||
const tnsr::IJ<DataVector, Dim>& /*stress*/) noexcept { | ||
for (size_t d = 0; d < Dim; d++) { | ||
source_for_displacement->get(d) = 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.
The commit message is incorrect. The systems don't need the fluxes to compute sources, but that having access to the fluxes is an optimization.