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

Add support for Unitful.jl numbers to MonotonicInterpolations #361

Merged
merged 9 commits into from
Oct 19, 2020

Conversation

MatFi
Copy link
Contributor

@MatFi MatFi commented Mar 26, 2020

Now one can do:

using Unitful
using Interpolations

x=-1u"s":0.25u"s":1u"s" ;
y=sign.(0.1u"s".+x)u"m";
itp = interpolate(x, y, FiniteDifferenceMonotonicInterpolation());
julia> itp(-0.125*u"s")
0.0 m

The reason why Unitful.jl did not work here before is that all polynomial coefficients were of the same type TCoeffs. When using Unitful numbers, the different dimensions of the coefficients become relevant. Therefore every coefficient now has its own type.
This pull request additionally contains a fix of a erroneous tangent calculation in FritschCarlsonMonotonicInterpolation.
(Be warned, I have been using julia only since version 1.0, so this may not be implemented optimally)

@timholy
Copy link
Member

timholy commented Apr 12, 2020

Sorry for the lack of review. If you're interested in a bigger role (see #365) I'll be happy to review this.

@MatFi
Copy link
Contributor Author

MatFi commented Apr 13, 2020

I'm honored and I'd love to, but giving a hack like me even a shred of responsibility for this package would be the end of it. I think the best way to give something back to the community is to stay on the ground where I'm reasonably proficient in what I do. Of course I like to help out where I can.

@timholy
Copy link
Member

timholy commented Apr 14, 2020

Sounds good. I'll leave this for my successor to review as good practice! Sorry for the likely delay.

@MatFi
Copy link
Contributor Author

MatFi commented Apr 14, 2020

No problem, I'm glad to hear that this PR becomes useful even before the merge

@mkitti
Copy link
Collaborator

mkitti commented Jun 25, 2020

Successor in training here. Could we develop some tests and add Unitful as a test dependency to ensure integration going forward? Also see #367

@timholy
Copy link
Member

timholy commented Jun 25, 2020

Note that this does add some Unitful tests (see xa and y). Unitful is already in the test deps, see https://github.com/JuliaMath/Interpolations.jl/blob/master/Project.toml

@timmyfaraday
Copy link

Is this feature available in the latest release, as I'm not able to recreate this behavior.
If not, any indication on when would it be available?

@mkitti
Copy link
Collaborator

mkitti commented Sep 15, 2020

Is this feature available in the latest release, as I'm not able to recreate this behavior.
If not, any indication on when would it be available?

This branch as not been merged yet. It's a high priority at the moment since @timholy already looked at it. I'll take a look this weekend and hopefully merge within two weeks.

@mkitti
Copy link
Collaborator

mkitti commented Oct 19, 2020

I intend to merge this and eventually release as v0.13.1

@mkitti mkitti merged commit 13da9c4 into JuliaMath:master Oct 19, 2020
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.

6 participants