-
Notifications
You must be signed in to change notification settings - Fork 192
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 new common time dependent options in Sphere and BCO domains #6115
base: develop
Are you sure you want to change the base?
Conversation
d576853
to
c37a13a
Compare
c37a13a
to
716e208
Compare
a6699f1
to
d82e058
Compare
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.
ok I'll need more coffee for this one
You can squash everything into one commit, except keep the "Allow FoTs to create themselves" commit separate.
std::array<DataVector, MaxDeriv + 1> all_derivs{}; | ||
for (size_t i = 0; i < all_derivs.size(); i++) { | ||
gsl::at(all_derivs, i) = std::move(all_derivs_vec[i]); | ||
} |
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 this is make_array<MaxDeriv+1>(func_and_all_derivs(t))
std::array<DataVector, MaxDeriv + 1> all_angle_derivs{}; | ||
for (size_t i = 0; i < all_angle_derivs.size(); i++) { | ||
gsl::at(all_angle_derivs, i) = std::move(all_angle_derivs_vec[i]); | ||
} |
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 make_array
struct TranslationMapOptions { | ||
using type = Options::Auto<TranslationMapOptions, Options::AutoLabel::None>; | ||
static std::string name() { return "TranslationMap"; } | ||
struct TranslationMapHardcodedOptions { |
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 always read "hardcore options" 💪
Suggestion: keep this class named TranslationMapOptions
. And the tag below name just TranslationMap
because it's already in the time_dependent_options
namespace.
Also this moves the FromVolumeFile option up a level in options so all the time dependent options are edited as well
5afe664
to
d98151f
Compare
Proposed changes
Final PR in the series that revamps the time dependent options for the Sphere and BCO domains.
This actually adds all the new common options to the domain creators.
Fixes #5584.
Upgrade instructions
The options for the Rotation and Expansion map in the BinaryCompactObject and Sphere domains have changed. In the BinaryCompactObject, they are now
In the Sphere domain, you can optionally specify the same options as the BinaryCompactObject like above to use functions of time that expire. Additionally, you can specify options to use SettleToConstant functions of time in the Sphere domain with the options below
Additionally, for any of the time dependent map options, you can choose the function of time from a volume H5 file by specifying the following options (not just for Expansion, but for Rotation and Translation as well and shape is slightly different)
Code review checklist
make doc
to generate the documentation locally intoBUILD_DIR/docs/html
.Then open
index.html
.code review guide.
bugfix
ornew feature
if appropriate.Further comments
Depends on and includes #6109, #6113, and #6114.