Skip to content

Commit 12fc4ea

Browse files
authored
Fix Derivative Update Bug (#264)
* Fix derivative macro bug. * fix tests * regex fix
1 parent 71a00ac commit 12fc4ea

File tree

4 files changed

+26
-3
lines changed

4 files changed

+26
-3
lines changed

docs/src/guide/derivative.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,12 @@ dydt2(t, ξ)
191191
This will also support anonymous definition and multi-dimensional definition.
192192
Please see [Macro Variable Definition](@ref) for more information.
193193

194+
!!! warning
195+
The same derivative should not be redefined with multiple `@variable` calls
196+
and using `@variable` to define derivatives should be avoided on derivatives
197+
that were already defined. This is because the latest `@variable` will
198+
overwrite any existing properties a derivative might already have.
199+
194200
Second, for more convenient definition we use [`@deriv`](@ref) (or [`@∂`](@ref))
195201
as shown in the Basic Usage section above. Unlike `@variable` this can handle any
196202
`InfiniteOpt` expression as the argument input. It also can build derivatives

docs/src/guide/variable.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
```@meta
2-
DocTestFilters = [r"≤|<=", r" == | = ", r" ∈ | in ", r" for all | ∀ "]
2+
DocTestFilters = [r"≤|<=", r" == | = ", r" ∈ | in ", r" for all | ∀ ", r"sin \(.*"]
33
```
44

55
# [Variables](@id var_docs)

src/derivatives.jl

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -332,7 +332,13 @@ function add_derivative(
332332
else
333333
dref = DerivativeRef(model, existing_index)
334334
gvref = _make_variable_ref(model, existing_index)
335+
old_info = _variable_info(dref)
336+
if old_info.has_lb || old_info.has_ub || old_info.has_fix || old_info.has_start
337+
@warn "Overwriting $dref, any previous properties (e.g., lower bound " *
338+
"or start value) will be lost/changed."
339+
end
335340
_update_info_constraints(d.info, gvref, dref)
341+
_set_core_variable_object(dref, d)
336342
if !isempty(name)
337343
set_name(dref, name)
338344
end

test/derivatives.jl

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,8 @@ end
206206
info = VariableInfo(false, num, false, num, false, num, false, NaN, false, false)
207207
info2 = VariableInfo(true, num, true, num, true, num, true, func1, false, false)
208208
info3 = VariableInfo(true, num, true, num, true, num, true, 42, false, true)
209-
info4 = VariableInfo(true, num, false, num, false, num, false, s -> NaN, false, false)
209+
info4 = VariableInfo(true, num, false, num, false, num, true, s -> NaN, false, false)
210+
info5 = VariableInfo(false, num, false, num, false, num, false, s -> NaN, false, false)
210211
# build_derivative
211212
@testset "build_derivative" begin
212213
# test errors
@@ -302,12 +303,22 @@ end
302303
idx = DerivativeIndex(1)
303304
dref = DerivativeRef(m, idx)
304305
gvref = InfiniteOpt._make_variable_ref(m, idx)
305-
d = Derivative(info4, true, x, pref)
306+
d = Derivative(info4, false, x, pref)
306307
@test isequal(add_derivative(m, d, "name2"), gvref)
307308
@test haskey(InfiniteOpt._data_dictionary(dref), idx)
308309
@test InfiniteOpt._core_variable_object(dref) == d
309310
@test name(dref) == "name2"
310311
@test lower_bound(dref) == 0
312+
# test redundant build with warning
313+
idx = DerivativeIndex(1)
314+
dref = DerivativeRef(m, idx)
315+
d = Derivative(info5, true, x, pref)
316+
warn = "Overwriting name2(pref, prefs), any previous properties (e.g., lower bound " *
317+
"or start value) will be lost/changed."
318+
@test_logs (:warn, warn) add_derivative(m, d, "name3")
319+
@test haskey(InfiniteOpt._data_dictionary(dref), idx)
320+
@test InfiniteOpt._core_variable_object(dref) == d
321+
@test name(dref) == "name3"
311322
end
312323
# test JuMP.add_variable
313324
@testset "JuMP.add_variable" begin

0 commit comments

Comments
 (0)