Skip to content

Commit

Permalink
fix: allow even more complex bindings
Browse files Browse the repository at this point in the history
One of the outstanding problems is a two model mutli-bind where

        B.K_1 -> A.K_1
        B.K_2 -> A.K_2
        A.K_2 -> A.K_1

in which case the bindings for `B` might come out as index `[1, 2]`, but
`A` only has 1 parameter in its parameter vector, leading to index out
of bounds.
  • Loading branch information
fjebaker committed Oct 4, 2024
1 parent 3f571de commit e0a9ea6
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 3 deletions.
6 changes: 5 additions & 1 deletion src/fitting/binding.jl
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,11 @@ function _construct_bound_mapping(bindings, parameter_count)
parameter_mapping[b[1]][b[2]] = reference[2]

# mark for removal: find the parameter index in the global array
N = sum(length(parameter_mapping[q]) for q = 1:b[1]-1)
N = if b[1] > 1
sum(length(parameter_mapping[q]) for q = 1:b[1]-1)
else
0
end
index = N + b[2]
push!(remove, index)

Expand Down
3 changes: 1 addition & 2 deletions src/xspec-models/convolutional.jl
Original file line number Diff line number Diff line change
Expand Up @@ -52,5 +52,4 @@ function XS_Kerrconv(;
XS_Kerrconv(Index1, Index2, r_br_g, a, Incl, Rin_ms, Rout_ms)
end

export XS_CalculateFlux,
XS_Kerrconv
export XS_CalculateFlux, XS_Kerrconv
19 changes: 19 additions & 0 deletions test/fitting/test-binding.jl
Original file line number Diff line number Diff line change
Expand Up @@ -123,3 +123,22 @@ new_bindings = SpectralFitting.adjust_free_bindings(prob.model, prob.bindings)
@test new_bindings == [[1 => 1, 2 => 3]]

# TODO: free parameters should not be allowed to bind to frozen parameters


# can we bind parameters within the same model
prob = FittingProblem(model1 => dummy_data1)
bind!(prob, 1 => :K_1, 1 => :a_1)
_, mapping = SpectralFitting._build_parameter_mapping(prob.model, prob.bindings)
@test mapping == ([1, 1, 2],)

# can we bind parameters within the same model
model1.a_2.frozen = false
prob = FittingProblem(model1 => dummy_data1, model1 => dummy_data1)
bind!(prob, 1 => :K_1, 2 => :K_1)
bind!(prob, 1 => :a_1, 2 => :a_1)
bind!(prob, 1 => :K_2, 2 => :K_2)
bind!(prob, 1 => :a_2, 2 => :a_2)
bind!(prob, 1 => :K_1, 1 => :a_1)
details(prob)
params, mapping = SpectralFitting._build_parameter_mapping(prob.model, prob.bindings)
@test mapping == ([1, 1, 2],)

0 comments on commit e0a9ea6

Please sign in to comment.