Skip to content

Export/import round trip fails for a Solution with unset optional solver fields (NotProvided) #464

Closed

Description

@ocots

Summary

A Solution that leaves the optional solver fields iterations and constraints_violation unset (they default to Core.NotProvided in build_solution) can be exported but not imported back, with both the JSON and the JLD2 formats. message and status have the same default and the same problem, except that the import does not fail: it silently brings back a different value (see below).

Solutions built by an indirect method are affected. Every Solution returned by a CTFlows OCP flow (Flow(ocp, law)((t0, tf), x0, p0)) is in this case, since an integration has no iteration count and no constraint violation. Found while working on CTModels#460 / CTFlows#443.

Reproducer (CTModels main, be2475f)

using CTModels: CTModels
using JSON3: JSON3
using JLD2: JLD2

pre = CTModels.Building.PreModel()
CTModels.Building.time_dependence!(pre; autonomous=true)
CTModels.Building.time!(pre; t0=0.0, tf=1.0)
CTModels.Building.state!(pre, 1)
CTModels.Building.control!(pre, 1)
CTModels.Building.dynamics!(pre, (r, t, x, u, v) -> (r[1] = u; nothing))
CTModels.Building.objective!(pre, :min; lagrange=(t, x, u, v) -> 0.5 * u^2)
ocp = CTModels.Building.build(pre)

T = collect(range(0.0, 1.0, 11))
# as CTFlows does: no `iterations`, no `constraints_violation`
sol = CTModels.Solutions.build_solution(
    ocp, T, t -> [t], t -> [1.0], Float64[], t -> [0.0];
    objective=0.5, message="m", status=:Success, successful=true,
)
CTModels.Solutions.iterations(sol)             # NotProvided
CTModels.Solutions.constraints_violation(sol)  # NotProvided

for fmt in (:JSON, :JLD)
    CTModels.Serialization.export_ocp_solution(sol; format=fmt, filename="sol")   # ok
    CTModels.Serialization.import_ocp_solution(ocp; format=fmt, filename="sol")   # fails
end

Output:

JSON: export ok
JSON: IMPORT FAILS: MethodError: no method matching Float64(::String)
JLD:  export ok
JLD:  IMPORT FAILS: MethodError: no method matching Float64(::CTBase.Core.NotProvidedType)

Cause

  • JSON. NotProvided is written as the string "NotProvided". The exported file contains "iterations": "NotProvided" and "constraints_violation": "NotProvided".

  • Reconstruction. src/Serialization/reconstruction_helpers.jl (both branches, around lines 72 and 109) assumes the fields are always set:

    iterations=data["iterations"],
    constraints_violation=Float64(data["constraints_violation"]),
    • Float64("NotProvided") (JSON) and Float64(NotProvided) (JLD2) throw a raw MethodError, not a typed exception (EXC1).
    • Even without the Float64, iterations="NotProvided" would not match Union{Int,NotProvidedType}.
  • message and status. The same reasoning applies to these two optional fields:

    • an unset status comes back as Symbol("NotProvided"), i.e. the symbol :NotProvided, not Core.NotProvided;
    • an unset message comes back as the string "NotProvided".

    The round trip does not fail but changes the value, which is worse. This is not checked yet.

Proposed fix

  1. Export. Write an unset optional field in an explicit, format-independent way: omit the key, or write JSON null / JLD2 nothing. Do not write the string "NotProvided".

  2. Import. Map a missing / null / nothing value back to Core.NotProvided (helper such as _optional(data, key, T)). For files written by the current version, also accept the legacy string "NotProvided".

  3. Errors. Throw a typed exception (Exceptions.ParsingError) for a value of the wrong type, instead of a MethodError.

  4. Fields covered. Treat message and status the same way as iterations and constraints_violation.

  5. Tests. Round-trip tests (JSON and JLD2) for:

    • a solution with all four optional fields unset;
    • a solution with all four set;
    • a legacy file containing "NotProvided".

    Each test checks iterations, constraints_violation, message and status after the import.

Notes

  • This is independent of CTModels#460 / feat(Solutions): switching structure of a solution; strict time grids #463 (the switching structure, SERIALIZATION_FORMAT_VERSION = 2). It can land before or after it. If it lands after, it can reuse the format version bump.
  • Downstream: once fixed, CTFlows can add an export/import round-trip test for the Solution of a multi-phase OCP flow (CTFlows#443).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

bugSomething isn't working

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions