From e0a9ea6c08bdcf5e27347f886640b4794754da88 Mon Sep 17 00:00:00 2001 From: fjebaker Date: Fri, 4 Oct 2024 12:30:42 +0100 Subject: [PATCH 1/2] fix: allow even more complex bindings 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. --- src/fitting/binding.jl | 6 +++++- src/xspec-models/convolutional.jl | 3 +-- test/fitting/test-binding.jl | 19 +++++++++++++++++++ 3 files changed, 25 insertions(+), 3 deletions(-) diff --git a/src/fitting/binding.jl b/src/fitting/binding.jl index e5aa4b7..319ff1c 100644 --- a/src/fitting/binding.jl +++ b/src/fitting/binding.jl @@ -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) diff --git a/src/xspec-models/convolutional.jl b/src/xspec-models/convolutional.jl index 1604a76..a00f8d6 100644 --- a/src/xspec-models/convolutional.jl +++ b/src/xspec-models/convolutional.jl @@ -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 diff --git a/test/fitting/test-binding.jl b/test/fitting/test-binding.jl index 9af4413..6b2bd02 100644 --- a/test/fitting/test-binding.jl +++ b/test/fitting/test-binding.jl @@ -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],) From 9d42930e185bbc60b5cd346e5f473d88b48222fc Mon Sep 17 00:00:00 2001 From: fjebaker Date: Fri, 4 Oct 2024 12:47:09 +0100 Subject: [PATCH 2/2] fixup! fix: allow even more complex bindings --- test/fitting/test-binding.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/fitting/test-binding.jl b/test/fitting/test-binding.jl index 6b2bd02..9078cd5 100644 --- a/test/fitting/test-binding.jl +++ b/test/fitting/test-binding.jl @@ -141,4 +141,4 @@ 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],) +@test mapping == ([1, 1, 2, 3], [1, 2, 2, 3])