Skip to content

ExaModelsOptimalControl ships but is never loaded — what is the intended direction? #323

Description

@ocots

cc @jbcaillau — you handled the earlier discussion with the ExaModels team on this, so you will know the intent better than I do.

What we observed

ext/ExaModelsOptimalControl.jl ships in the package but is not listed in [extensions], so Julia never loads it. Everything in it is therefore unreachable: linear algebra on expression nodes (dot, * for node vectors and matrices, det, norm, tr, diag), convert / promote_rule / zero / one for AbstractNode, and the Null zero/one elimination.

using ExaModels, LinearAlgebra

x, y = ExaModels.Null(1.0), ExaModels.Null(2.0)

dot([1, 2], [x, y])
# ERROR: MethodError: no method matching iterate(::ExaModels.Null{Float64})

convert(ExaModels.AbstractNode, 5)
# ERROR: MethodError: Cannot `convert` an object of type Int64 to an object of type
#        ExaModels.AbstractNode

Both work as soon as the shipped file is loaded by hand:

module OptimalControl end   # stand in for the trigger
p = joinpath(dirname(dirname(pathof(ExaModels))), "ext", "ExaModelsOptimalControl.jl")
src = replace(read(p, String),
              "using ExaModels, LinearAlgebra, OptimalControl" => "using ExaModels, LinearAlgebra")
include_string(Main, src)

dot([1, 2], [x, y])                 # Null(5.0)
convert(ExaModels.AbstractNode, 5)  # Null(5)

So the code itself is current and correct against 0.12 — it is only unwired. This is the state of main as well as of the 0.12.0 release.

Two details suggest this was not deliberate. LinearAlgebra is still in [weakdeps] with no [extensions] entry referencing it — the leftover from 0.9.7, where the same file was registered as ExaModelsLinearAlgebra = "LinearAlgebra". And OptimalControl is not in [weakdeps] at all, so the currently intended trigger could not fire even if the [extensions] line were added on its own.

What we are actually asking

We would rather understand the plan than send a patch that cuts across it. A few directions look plausible from the outside, and we do not know which one you are on:

  1. Keep it and wire it up as-is — add OptimalControl to [weakdeps] and the matching [extensions] entry.
  2. Re-trigger it on LinearAlgebra, as in 0.9.7. Nothing in the file uses OptimalControl; it is generic linear algebra over ExaModels' own node types, needed by anyone building expressions out of arrays. That would also explain the leftover weakdep.
  3. Retire it from ExaModels and let downstream packages own this glue. Perfectly reasonable — but then we would like to know, so we stop treating our copy as temporary.
  4. Evolve it into something with a different shape or scope that you would rather we build against.

Happy to open a PR for whichever of these you prefer.

Why we ran into it

We are moving CTParser.jl to ExaModels 0.12 (control-toolbox/CTParser.jl#325). CTParser generates ExaModels code from a DSL, so a user writing

(x)(t) == A * x(t) + B * u(t)
(dot(q, x(t)))  min

ends up applying * and dot to arrays of ExaModels nodes while the model is being built. Without those methods the model cannot be constructed at all.

For now we carry a port of the file in our own extension, triggered by ExaModels + LinearAlgebra, so our users are not blocked. We would prefer to delete it — hence the question above rather than a patch.

A separate ambiguity, related only by cause

Once the Null zero elimination is active (structural zeros fold to Null, so both operands of a second-order adjoint pass can be SecondAdjointNull at once), this becomes reachable in src/, not in the extension:

MethodError: _hdrpass_val(::Type{SecondAdjointNull{Float64}},
                          ::Type{SecondAdjointNull{Float64}}) is ambiguous.

Candidates:
  _hdrpass_val(::Type{<:SecondAdjointNull}, ::Type)   @ src/simdfunction.jl:142
  _hdrpass_val(::Type, ::Type{<:SecondAdjointNull})   @ src/simdfunction.jl:143

We hit it on a second derivative of a dynamics written with dot, under a trapeze discretisation. Both declared methods return Val(0), so the value at the intersection looks forced:

_hdrpass_val(::Type{<:SecondAdjointNull}, ::Type{<:SecondAdjointNull}) = Val(0)

We are working around it locally. Say the word and we will split it into its own issue — it stands on its own, and is only reachable because of the extension question above.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions