-
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 functionality to Ylm IO #5921
Add functionality to Ylm IO #5921
Conversation
iter.reset(); | ||
|
||
// Where max_l is larger than strahlkorper.l_max() | ||
{ |
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.
[optional] My understanding is that the goal of this function is that the output is effectively a prolonged version of the input strahlkorper to the requested l_max. If so, a simpler test for this part would be to check that fill_ylm_legend_and_data(strahkorper, larger_l_max)
and fill_ylm_legend_and_data(Strahlkorper(larger_l_max, larger_l_max, strahkorper), larger_l_max)
give the same result.
* \param time Time to read the coefficients at. | ||
* \param epsilon How much error is allowed when looking for a specific time. | ||
* This is useful so users don't have to know the specific time to machine | ||
* precision. |
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.
What is the expected use case where the caller doesn't know the exact time?
Mention that epsilon
is a relative error, possibly by renaming it to indicate that.
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.
Say I've been dumping data every 0.5M and I want to read it it at time 123.5M. However, the actual time of the output is something like 123.499999999999748. The epsilon accounts for this. The rename sounds like a good idea though
@wthrowe pushed fixups |
OK. Squash and rebase. |
ffd02c6
to
76d7736
Compare
Proposed changes
This is the first of 3 PRs to allow reading horizon coefficients from a file into the shape map coefficients in a domain creator. This PR just factors out a function into a more accessible part of the code and adds the ability to read Ylm coefs at a specific time from a file.
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