-
Notifications
You must be signed in to change notification settings - Fork 191
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 YlmsFromSpEC to shape time dependent options #6114
Add YlmsFromSpEC to shape time dependent options #6114
Conversation
6297c70
to
dc63f9c
Compare
dc63f9c
to
2ee9eb8
Compare
No longer dependent. Also added a commit that adds |
2ee9eb8
to
7938487
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.
LGTM. A few small requests. Please squash immediately
7938487
to
6e22f9b
Compare
@nilsdeppe Squashed everything in. Thanks for the review |
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.
LGTM! Thanks for doing this :D
Wat, clang-tidy now doesn't want you to use Edit: the |
6e22f9b
to
0b9ec46
Compare
Done! |
Proposed changes
This is tangentially related to the PRs revamping the time dependent options for the Sphere and BCO domain creators.
This adds the ability to read the shape coefficients from a SpEC ID horizons dat file (not a dat file within an H5 file, but an actual dat file). If we are starting from SpEC ID (which we can do), then this is an important feature to have.
Upgrade instructions
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.