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

Move to Interpolations.jl #44

Merged
merged 4 commits into from
Jul 5, 2024
Merged

Move to Interpolations.jl #44

merged 4 commits into from
Jul 5, 2024

Conversation

Sbozzolo
Copy link
Member

@Sbozzolo Sbozzolo commented Jul 3, 2024

Note that the order of interpolation is now first instead of cubic.

Is this acceptable?

Closes #43
Closes #39

@charleskawczynski
Copy link
Member

Hmm, the docs are breaking, and the tests have all sorts of issues. Maybe we can fix the tests first.

@Sbozzolo
Copy link
Member Author

Sbozzolo commented Jul 3, 2024

Hmm, the docs are breaking, and the tests have all sorts of issues. Maybe we can fix the tests first.

What issues do you see in tests? Aren't they all passing?

@Sbozzolo
Copy link
Member Author

Sbozzolo commented Jul 3, 2024

The error in the docs is because by default Interpolations.jl does not allow extrapolation, and something is trying to evaluate a point outside of the domain of defition of the interpolant.

@charleskawczynski
Copy link
Member

What issues do you see in tests? Aren't they all passing?

Sorry, I meant that the design of the tests in this repo need improvement-- Right now, the things that are tested are implicit, based on names(AtmosphericProfilesLibrary; all = true) / getproperty(AtmosphericProfilesLibrary, name) of those names, which requires ignoring/skipping names-- it's way more complicated than it needs to be. Also, the inference tests are just wrong (@test @inferred Float32 iscallable(prof(Float32))).

@charleskawczynski
Copy link
Member

I think #45 fixes all of this.

@charleskawczynski
Copy link
Member

Sorry about all the conflicts, I can rebase if you'd like

@charleskawczynski
Copy link
Member

Rebased

Copy link
Member

@charleskawczynski charleskawczynski left a comment

Choose a reason for hiding this comment

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

Results in the docs look visually indistinguishable.

@charleskawczynski charleskawczynski merged commit db4baac into main Jul 5, 2024
6 checks passed
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.

Switch from Dierckx.jl to Interpolations.jl
2 participants