-
Notifications
You must be signed in to change notification settings - Fork 240
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
New Akima spline component for interpolation. #919
Conversation
|
||
The following is a simple example where an AkimaSplineComp is used to approximate a curve that has | ||
11 points where we would like to evaluate it. The approximating curve contains 6 points. Note that | ||
unlke the `BsplinesComp`, the control points fall on the curve. |
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 makes me wonder if the component should be AkimaSplinesComp (plural) for consistency.
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 I'd rather rename the Bsplines Comp, but I am not sure. Can you guys have Tristan weigh in on the proper name for this component?
desc="half-width of the smoothing interval added in the valley of " | ||
"absolute-value function. This allows the derivatives with respect to" | ||
" the data points (dydxpt, dydypt) to also be C1 continuous. Set to " | ||
"parameter to 0 to get the original Akima function (but only if you " |
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.
typo: "Set to parameter to 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.
fixed
"to end, compute num_points values spanning the full interval set by " | ||
"the training points. When set to 'cell_center', compute values at " | ||
"centerpoints of num_points cells. Only used if xcp_name is not " | ||
"given.") |
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.
It looks like this should say "Only used if 'input_x' is not True"... (xcp_name is not something the user provides)
Also, a couple of editorial notes:
- "evaluate" -> "evaluated"
- should have single quotes around 'end' (like for 'cell_center')
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.
Fixed. Leftover from the old API.
self.options.declare('vec_size', types=int, default=1, | ||
desc='Number of independent rows to interpolate.') | ||
self.options.declare('name', types=str, default=None, allow_none=True, | ||
desc="Name to use for the interpolated variable.") |
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.
can 'name' really be None? It looks like it's used to create the input and output vars...
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.
Fixed. Fixed all but question about what to call the component.
No description provided.