diff --git a/CHANGELOG.md b/CHANGELOG.md index 8bbacc2b..32401d4a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -50,6 +50,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `Solution` gains an eleventh field and type parameter, `switching`; the ten-argument positional constructor is kept and builds an `EmptySwitchingModel`. +### 🔧 Fixed + +- **Export/import round trip of a `Solution` with unset optional solver fields** + ([#464](https://github.com/control-toolbox/CTModels.jl/issues/464)). A solution whose + `iterations`, `constraints_violation`, `message` or `status` are `NotProvided` (as the + ones built by an indirect method) could be exported but not imported, in JSON and JLD2. + An unset field is now written as `null` (JSON) / `nothing` (JLD2) and read back as + `NotProvided`; files holding the string `"NotProvided"` (older versions) are still read. + A value of the wrong type raises a `ParsingError` instead of a `MethodError`. + ### 📚 Documentation - New page *Switching structure*; the `build_solution` contract, plotting and diff --git a/ext/CTModelsJLD.jl b/ext/CTModelsJLD.jl index fa9d75f2..f25d3f7b 100644 --- a/ext/CTModelsJLD.jl +++ b/ext/CTModelsJLD.jl @@ -84,7 +84,13 @@ julia> using JLD2, CTModels julia> sol = CTModels.import_ocp_solution(CTModels.JLD2Tag(), model; filename="mysolution") ``` +# Throws +- `CTBase.Exceptions.ParsingError`: if an optional solver field has a value of the wrong type. + # Notes +- An unset optional solver field (`iterations`, `constraints_violation`, `message`, `status`) + is stored as `nothing` and read back as `Core.NotProvided` (`Core.NotProvided` stored by + older versions is read the same way) - The solution is reconstructed from discretized data via `build_solution` - This ensures perfect round-trip consistency with the export - The file does not contain the model: `ocp` must be the model the solution was computed diff --git a/ext/CTModelsJSON.jl b/ext/CTModelsJSON.jl index e50bae3b..b4f7cbaa 100644 --- a/ext/CTModelsJSON.jl +++ b/ext/CTModelsJSON.jl @@ -296,8 +296,13 @@ including the discretized primal and dual trajectories. - `CTBase.Exceptions.IncorrectArgument`: if the file does not match `ocp` (dimensions, number or labels of constraints, fixed versus free times). Pass the model the solution was computed from. +- `CTBase.Exceptions.ParsingError`: if an optional solver field has a value of the wrong type. # Notes +An unset optional solver field (`iterations`, `constraints_violation`, `message`, `status`) +is written as `null` and read back as `Core.NotProvided`; the string `"NotProvided"`, written +by older versions, is read the same way. + Handles both vector and matrix encodings of signals. If dual fields are missing or `null`, the corresponding attributes are set to `nothing`. The file is checked against `ocp` before the solution is rebuilt; see @@ -332,10 +337,10 @@ function CTModels.import_ocp_solution( data = Dict{String,Any}( "objective" => blob["objective"], - "iterations" => blob["iterations"], - "constraints_violation" => blob["constraints_violation"], - "message" => blob["message"], - "status" => blob["status"], + "iterations" => get(blob, "iterations", nothing), + "constraints_violation" => get(blob, "constraints_violation", nothing), + "message" => get(blob, "message", nothing), + "status" => get(blob, "status", nothing), "successful" => blob["successful"], "state" => _json_to_matrix(blob["state"]), "control" => _json_to_matrix(blob["control"]), diff --git a/src/Serialization/reconstruction_helpers.jl b/src/Serialization/reconstruction_helpers.jl index 91e843ea..d7063413 100644 --- a/src/Serialization/reconstruction_helpers.jl +++ b/src/Serialization/reconstruction_helpers.jl @@ -4,6 +4,34 @@ """ $(TYPEDSIGNATURES) +Read the optional solver field `key` of `data` as a `T`, or `Core.NotProvided` if it is unset. + +A field is unset when its key is missing or when its value is `nothing` (`null` in JSON), +`Core.NotProvided`, or the string `"NotProvided"` (written by older versions). + +# Throws +- `CTBase.Exceptions.ParsingError`: if the value cannot be converted to `T`. +""" +function _optional(data, key::String, ::Type{T}) where {T} + value = get(data, key, nothing) + (isnothing(value) || value isa CTBase.Core.NotProvidedType || value == "NotProvided") && + return CTBase.Core.NotProvided + try + return T(value) + catch + throw( + Exceptions.ParsingError( + "solution field '$key' must be a $T, got $(repr(value))"; + location="imported solution dictionary", + suggestion="re-export the solution with a current CTModels version", + ), + ) + end +end + +""" +$(TYPEDSIGNATURES) + Reconstruct a solution from imported data, detecting the format (single vs multiple time grids). Duals and `control_interpolation` are read from `data`. Only `infos` is accepted as a @@ -73,10 +101,10 @@ function _reconstruct_solution_from_data(ocp, data; infos=Dict{Symbol,Any}()) _extract_time_vector(data["variable"]), data["costate"]; objective=Float64(data["objective"]), - iterations=data["iterations"], - constraints_violation=Float64(data["constraints_violation"]), - message=data["message"], - status=Symbol(data["status"]), + iterations=_optional(data, "iterations", Int), + constraints_violation=_optional(data, "constraints_violation", Float64), + message=_optional(data, "message", String), + status=_optional(data, "status", Symbol), successful=data["successful"], path_constraints_dual=path_constraints_dual, boundary_constraints_dual=boundary_constraints_dual, @@ -111,10 +139,10 @@ function _reconstruct_solution_from_data(ocp, data; infos=Dict{Symbol,Any}()) _extract_time_vector(data["variable"]), data["costate"]; objective=Float64(data["objective"]), - iterations=data["iterations"], - constraints_violation=Float64(data["constraints_violation"]), - message=data["message"], - status=Symbol(data["status"]), + iterations=_optional(data, "iterations", Int), + constraints_violation=_optional(data, "constraints_violation", Float64), + message=_optional(data, "message", String), + status=_optional(data, "status", Symbol), successful=data["successful"], path_constraints_dual=path_constraints_dual, boundary_constraints_dual=boundary_constraints_dual, diff --git a/src/Solutions/build_solution.jl b/src/Solutions/build_solution.jl index 1fb6bb76..475db59a 100644 --- a/src/Solutions/build_solution.jl +++ b/src/Solutions/build_solution.jl @@ -1728,6 +1728,15 @@ const SERIALIZATION_FORMAT_VERSION = 2 """ $(TYPEDSIGNATURES) +Return `nothing` for an unset optional field (`Core.NotProvided`), and `x` otherwise. + +An unset field is written as `nothing` (`null` in JSON), whatever the format. +""" +_unset_to_nothing(x) = x isa Core.NotProvidedType ? nothing : x + +""" +$(TYPEDSIGNATURES) + Return the key of the serialized `side` limit (`1` left, `2` right) of `component`, e.g. `"switching_state_left"`. """ @@ -1845,7 +1854,9 @@ Dict( - `"control_constraints_lb_dual"`, `"control_constraints_ub_dual"`: Control box duals on control grid - `"boundary_constraints_dual"`: Boundary duals (time-independent vector) - `"variable_constraints_lb_dual"`, `"variable_constraints_ub_dual"`: Variable duals (vectors) - - **Solver info**: `"iterations"`, `"message"`, `"status"`, `"successful"`, `"constraints_violation"`, `"infos"` + - **Solver info**: `"iterations"`, `"message"`, `"status"`, `"successful"`, `"constraints_violation"`, `"infos"`. + The optional fields `"iterations"`, `"message"`, `"status"` and `"constraints_violation"` are + `nothing` when unset (`Core.NotProvided`) - **Compatibility check**: `"format_version"` and `"model_signature"` (see [`CTModels.Models._model_signature`](@extref)), used at import time to check the file against the model @@ -2005,11 +2016,11 @@ function _discretize_all_components( "boundary_constraints_dual" => boundary_constraints_dual(sol), "variable_constraints_lb_dual" => _as_vector(variable_constraints_lb_dual(sol)), "variable_constraints_ub_dual" => _as_vector(variable_constraints_ub_dual(sol)), - "iterations" => iterations(sol), - "message" => message(sol), - "status" => status(sol), + "iterations" => _unset_to_nothing(iterations(sol)), + "message" => _unset_to_nothing(message(sol)), + "status" => _unset_to_nothing(status(sol)), "successful" => successful(sol), - "constraints_violation" => constraints_violation(sol), + "constraints_violation" => _unset_to_nothing(constraints_violation(sol)), "infos" => infos(sol), "format_version" => SERIALIZATION_FORMAT_VERSION, "model_signature" => Models._model_signature(model(sol)), diff --git a/test/suite/serialization/test_optional_fields.jl b/test/suite/serialization/test_optional_fields.jl new file mode 100644 index 00000000..9d00b2d5 --- /dev/null +++ b/test/suite/serialization/test_optional_fields.jl @@ -0,0 +1,143 @@ +module TestOptionalFields + +using Test: Test +using JSON: JSON +using JLD2: JLD2 +using CTBase: CTBase +using CTModels: Building +using CTModels: Solutions +using CTModels: Serialization + +const VERBOSE = isdefined(Main, :TestData) ? Main.TestData.VERBOSE : true +const SHOWTIMING = isdefined(Main, :TestData) ? Main.TestData.SHOWTIMING : true + +const NP = CTBase.Core.NotProvided + +const FORMATS = (:JSON, :JLD) +const FIELDS = (:iterations, :constraints_violation, :message, :status) + +function _ocp() + pre = Building.PreModel() + Building.time_dependence!(pre; autonomous=true) + Building.time!(pre; t0=0.0, tf=1.0) + Building.state!(pre, 1) + Building.control!(pre, 1) + Building.dynamics!(pre, (r, t, x, u, v) -> (r[1] = u; nothing)) + Building.objective!(pre, :min; lagrange=(t, x, u, v) -> 0.5 * u^2) + return Building.build(pre) +end + +function _solution(ocp; set::Bool) + T = collect(range(0.0, 1.0, 11)) + extra = if set + (; iterations=7, constraints_violation=1e-9, message="converged", status=:Success) + else + (;) + end + return Solutions.build_solution( + ocp, T, t -> [t], t -> [1.0], Float64[], t -> [0.0]; + objective=0.5, successful=true, extra..., + ) +end + +_values(sol) = map(f -> getfield(Solutions, f)(sol), FIELDS) + +_ext(fmt) = fmt === :JSON ? ".json" : ".jld2" + +# What the previous version wrote for an unset field. +function _make_legacy(fmt, path) + if fmt === :JSON + blob = JSON.parse(read(path, String); dicttype=Dict{String,Any}) + for k in String.(FIELDS) + blob[k] = "NotProvided" + end + write(path, JSON.json(blob; pretty=true)) + else + data = JLD2.load(path)["solution_data"] + data["iterations"] = NP + data["constraints_violation"] = NP + data["message"] = NP + data["status"] = NP + JLD2.jldsave(path; solution_data=data) + end + return nothing +end + +function _import_with(ocp, fmt, path) + return Serialization.import_ocp_solution(ocp; format=fmt, filename=path) +end + +function test_optional_fields() + Test.@testset "Optional solver fields round trip (#464)" verbose = VERBOSE showtiming = + SHOWTIMING begin + ocp = _ocp() + mktempdir() do dir + for fmt in FORMATS + Test.@testset "$fmt" begin + path = joinpath(dir, "sol_$(fmt)") + file = path * _ext(fmt) + + Test.@testset "all four fields unset" begin + sol = _solution(ocp; set=false) + Test.@test all(v -> v === NP, _values(sol)) + Serialization.export_ocp_solution(sol; format=fmt, filename=path) + back = _import_with(ocp, fmt, path) + Test.@test all(v -> v === NP, _values(back)) + Test.@test Solutions.successful(back) + end + + Test.@testset "all four fields set" begin + sol = _solution(ocp; set=true) + Serialization.export_ocp_solution(sol; format=fmt, filename=path) + back = _import_with(ocp, fmt, path) + Test.@test Solutions.iterations(back) == 7 + Test.@test Solutions.constraints_violation(back) == 1e-9 + Test.@test Solutions.message(back) == "converged" + Test.@test Solutions.status(back) === :Success + end + + Test.@testset "legacy file with \"NotProvided\"" begin + sol = _solution(ocp; set=false) + Serialization.export_ocp_solution(sol; format=fmt, filename=path) + _make_legacy(fmt, file) + back = _import_with(ocp, fmt, path) + Test.@test all(v -> v === NP, _values(back)) + end + + Test.@testset "wrong type raises a ParsingError" begin + sol = _solution(ocp; set=true) + Serialization.export_ocp_solution(sol; format=fmt, filename=path) + if fmt === :JSON + blob = JSON.parse(read(file, String); dicttype=Dict{String,Any}) + blob["iterations"] = "many" + write(file, JSON.json(blob; pretty=true)) + else + data = JLD2.load(file)["solution_data"] + data["iterations"] = "many" + JLD2.jldsave(file; solution_data=data) + end + Test.@test_throws CTBase.Exceptions.ParsingError _import_with( + ocp, fmt, path + ) + end + end + end + + Test.@testset "JSON file holds null, not \"NotProvided\"" begin + path = joinpath(dir, "null_check") + Serialization.export_ocp_solution( + _solution(ocp; set=false); format=:JSON, filename=path + ) + blob = JSON.parse(read(path * ".json", String); dicttype=Dict{String,Any}) + for k in String.(FIELDS) + Test.@test haskey(blob, k) && blob[k] === nothing + end + end + end + end + return nothing +end + +end # module TestOptionalFields + +test_optional_fields() = TestOptionalFields.test_optional_fields()