Skip to content
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

Added Differentiate3 to Interpolation #1093

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

osswaldo
Copy link

No description provided.

@osswaldo osswaldo force-pushed the interpolation_differentiate3 branch from 74febe3 to 3cc5b65 Compare September 10, 2024 14:37
@jkalias
Copy link
Member

jkalias commented Sep 21, 2024

I would like to see some unit tests for this.

@osswaldo
Copy link
Author

I tool a closer look into the tests and there aren't any tests for the Differentiate methods at all.

@jkalias
Copy link
Member

jkalias commented Sep 24, 2024

Maybe we can start now having tests for it? ☺️
I don’t mean ALL the existing methods of course, just this new addition.

@osswaldo osswaldo force-pushed the interpolation_differentiate3 branch from 3cc5b65 to 6115680 Compare September 24, 2024 12:26
@osswaldo
Copy link
Author

Ok, done ;)

@osswaldo osswaldo force-pushed the interpolation_differentiate3 branch from 6115680 to 243d516 Compare September 24, 2024 12:28
@jkalias
Copy link
Member

jkalias commented Sep 28, 2024

This is awesome, thank you

/// </summary>
/// <param name="t">Point t to interpolate at.</param>
/// <returns>Interpolated third derivative at point t.</returns>
double Differentiate3(double t);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding a method to IInterpolation is a breaking change since IInterpolation is a public interface that consumers of the library can and do implement.
Might be better to leave it out of the interface but keep the method for the classes for which the method is supported.

@jkalias
Copy link
Member

jkalias commented Sep 28, 2024

It’s been more than 2 years that the latest 5.0 release of the library has been published. I’m not the official maintainer but I wouldn’t have something against releasing a 5.1 version with this change among others (it’s adding a feature, not removing existing ones or changing functionality)

@osswaldo
Copy link
Author

It isn't a realy "breaking" change if one extend an interface. You can simply implement the method as not implemented. However, it could be make sence to introduce an IDifferentiableInterpolation in order to remove the SupportsDifferentiation property.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants