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.

Sign up for GitHub

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

Migrate away from Dierckx.jl #54

Merged
merged 7 commits into from
Feb 13, 2025
Merged

Migrate away from Dierckx.jl #54

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
bdb6fc3
Changed package used for interpolations
icweaver Feb 12, 2025
55dd9ca
explicit > implicit
icweaver Feb 13, 2025
cbc93ee
Update Project.toml
icweaver Feb 13, 2025
114d5b8
Merge branch 'master' into interpolations
icweaver Feb 13, 2025
29f3053
inlined F19 data
icweaver Feb 13, 2025
7ccbb9c
finished sentence in comment
icweaver Feb 13, 2025
3e12464
dropped datadep registration
icweaver Feb 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions Project.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ license = "MIT"
version = "0.11.1"

[deps]
BSplineKit = "093aae92-e908-43d7-9660-e50ee39d5a0a"
DataDeps = "124859b0-ceae-595e-8997-d05f6a7a8dfe"
DelimitedFiles = "8bb1440f-4735-579b-a4ab-409b98df4dab"
Dierckx = "39dd38d3-220a-591b-8e3c-4c3a8c710a94"
FITSIO = "525bcba6-941b-5504-bd06-fd0dc1a4d2eb"
Parameters = "d96e819e-fc66-5662-9728-84c9c7592b0a"
Unitful = "1986cc42-f94f-5a68-af5c-568840ba703d"
Expand All @@ -15,7 +15,6 @@ UnitfulAstro = "6112ee07-acf9-5e0f-b108-d242c714bf9f"
[compat]
DataDeps = "0.7"
icweaver marked this conversation as resolved.
Show resolved Hide resolved
DelimitedFiles = "1"
Dierckx = "0.4, 0.5"
FITSIO = "0.13.0, 0.14, 0.15, 0.16.1, 0.17"
Parameters = "0.12"
Unitful = "0.17.0, 1"
Expand Down
2 changes: 1 addition & 1 deletion src/color_laws.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using Dierckx
using BSplineKit
using DelimitedFiles
cgarling marked this conversation as resolved.
Show resolved Hide resolved
icweaver marked this conversation as resolved.
Show resolved Hide resolved

# Convenience function for wavelength conversion
Expand Down
8 changes: 5 additions & 3 deletions src/color_laws/fitzpatrick.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ function _curve_F99_method(
y_splineval_optir = (0.0, optnir_axav_y...)
spline_x = (x_splineval_optir..., f99_x_splineval_uv...)
spline_y = (y_splineval_optir..., y_splineval_uv...)
spl = Spline1D(collect(spline_x), collect(spline_y), k=3)
spl = interpolate(collect(spline_x), collect(spline_y), BSplineOrder(4))
axav = spl(x)
end

Expand Down Expand Up @@ -262,7 +262,9 @@ and Rv. For more information, seek the original paper.
"""
function f19_invum(x::Real, Rv::Real)
# read and unpack tabulated data
data_x, data_k, data_delta_k, data_sigma_k = let data = readdlm(joinpath(datadep"F19", "F19_tabulated.dat"), skipstart=1)
# using type annotations so `interpolate` can be inferred
# TODO: Avoid using readdlm to avoid this issue altogether
data_x::Vector{Float64}, data_k::Vector{Float64}, data_delta_k::Vector{Float64}, data_sigma_k::Vector{Float64} = let data = readdlm(joinpath(datadep"F19", "F19_tabulated.dat"), skipstart=1)
(data[:, i] for i in 1:4)
end
Copy link
Member

Choose a reason for hiding this comment

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

With the caveat that I'm not familiar with this code, it seems to me it is inefficient to re-read the data for every call to f19_invum.

Why don't we load it as a constant (something like const f19_data) at the module level? I guess that would force download the datadep when you load the package but that sounds fine to me. That would also allow it's type to be known.

Frankly the data file's not even that large so we could just copy-paste the raw data into the source file unless there's some licensing issue or something ...

Copy link
Member Author

Choose a reason for hiding this comment

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

100% agree. I was very tempted to just inline it while writing out that TODO. Will play around with some schemes later tn


Expand All @@ -275,7 +277,7 @@ function f19_invum(x::Real, Rv::Real)
k_rV_tab_x = @. data_k + data_delta_k * (Rv - 3.10) * 0.990

# setup spline interpolation
spl = Spline1D(collect(data_x), collect(k_rV_tab_x), k=3)
spl = interpolate(collect(data_x), collect(k_rV_tab_x), BSplineOrder(4))

# use spline interpolation to evaluate the curve for the input x values
k_rV = spl(x)
Expand Down
4 changes: 2 additions & 2 deletions src/color_laws/m14.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@ const m14_b3v = @. (
const m14_xn = collect((m14_x1..., m14_x2..., m14_x3...))
const m14_anv = collect((m14_a1v..., m14_a2v..., m14_a3v...))
const m14_bnv = collect((m14_b1v..., m14_b2v..., m14_b3v...))
const m14_a_spl = Spline1D(m14_xn, m14_anv, bc="nearest")
const m14_b_spl = Spline1D(m14_xn, m14_bnv, bc="nearest")
const m14_a_spl = interpolate(m14_xn, m14_anv, BSplineOrder(4), Natural())
const m14_b_spl = interpolate(m14_xn, m14_bnv, BSplineOrder(4), Natural())

"""
M14(;Rv=3.1)
Expand Down
Loading