-
Notifications
You must be signed in to change notification settings - Fork 36
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
Adaptive step improvement #61
Conversation
@Agent6-6-6 if you have a chance, wondering if you'd mind having a quick look at this tosee what you think before I merge. Cheers! |
FYI there's a test documentation build here. P.S. the addition of the failure point was a very good suggestion, I think it works well! |
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.
@robbievanleeuwen take a look at the comments and see what you think!
Note I have not run any examples but the code appears to address all of the points that were raised in #51 and gives good granular control over the resulting curve if the defaults are not cutting it!
@robbievanleeuwen, wonder as well in the notebook example, perhaps an example illustrating the use of these new parameters to tune the accuracy of the generated curve? To show for example a default analysis vs one that is a bit more refined, perhaps demonstrating the tradeoff in terms of accuracy vs analysis time. But ultimately shows that limiting curvature is the same if that's all the user is interested in. |
Example modified, see here. Didn't want to make it too complex as the docs are already taking a fair bit of time to build 😆 |
@robbievanleeuwen example looks good, only suggestion would be to maybe look at adding some intermediate side bars to spice up the curve and have multiple yield points to further accentuate any differences between refined and default parameters? |
Fixes #51