Remove Index type

[ocots]

@jbcaillau

• I have remove Index type from range constraints and replaced it by Int.
• I have replaced Index for initial and final free times by Val. To remove this

CTBase.jl/src/types.jl

Lines 1154 to 1172 in 285afa2

	mutable struct Index
	val::Int
	Index(v::Int) = v ≥ 1 ? new(v) : error("index must be at least 1")
	end
	Base.:(==)(i::Index, j::Index) = i.val == j.val # needed, as this is not the default behaviour for composite types
	Base.to_index(i::Index) = i.val
	Base.isless(i::Index, j::Index) = i.val ≤ j.val
	Base.isless(i::Index, j::Real) = i.val ≤ j
	Base.isless(i::Real, j::Index) = i ≤ j.val
	Base.length(i::Index) = 1
	Base.iterate(i::Index, state = 0) = state == 0 ? (i, 1) : nothing
	Base.IteratorSize(::Type{Index}) = Base.HasLength()
	Base.append!(v::Vector, i::Index) = Base.append!(v, i.val)
	Base.getindex(x::AbstractArray, i::Index) = Base.getindex(x, i.val)
	Base.getindex(x::Real, i::Index) = x[i.val]
	# to suppress ambiguities
	Base.getindex(x::SparseArrays.ReadOnly, i::CTBase.Index) = Base.getindex(x, i.val)
	Base.getindex(x::StaticArrays.TrivialView, i::CTBase.Index) = Base.getindex(x, i.val)

I have made the choice to return an Int if the initial time is a Val. See

CTBase.jl/src/optimal_control_model-getters.jl

Line 485 in a27a83b

initial_time(ocp::OptimalControlModel) = __get_value(ocp.initial_time)

and

CTBase.jl/src/utils.jl

Lines 259 to 261 in a27a83b

	__get_value(x::Float64) = x
	__get_value(n::Int64) = n
	__get_value(::Val{N}) where {N} = N

This should imply no changes in CTDirect.jl since we can see how @PierreMartinon does here:

https://github.com/control-toolbox/CTDirect.jl/blob/1433c9a87430cd75ab21cb9a366675f115f4772c/src/utils.jl#L30-L36

[github-actions]

Breakage test results
Date: 2024-09-18 22:10:14

Package name	latest	stable
CTDirect.jl	compat: v0.13.1	compat: v0.13.1
CTFlows.jl	compat: v0.13.1	compat: v0.13.1
OptimalControl.jl	compat: v0.13.1	compat: v0.13.1

[ocots]

@jbcaillau On pourrait aussi avoir dans le OptimalControlModel, un champ pour l'indice :

@with_kw mutable struct OptimalControlModel{
    time_dependence <: TimeDependence,
    variable_dependence <: VariableDependence,
} <: AbstractOptimalControlModel
    initial_time::Union{Time, Val, Nothing} = nothing
    initial_time_index::Union{Int, Nothing} = nothing
    final_time::Union{Time, Val, Nothing} = nothing
    final_time_index::Union{Int, Nothing} = nothing
...

et avoir les getters associés. On pourrait aussi fournir une méthode :

function initial_time(ocp::OptimalControlModel{<:TimeDependence, NonFixed}, v::Variable)
    return v[ocp.initial_time_index]
end

et pareil pour le temps final.

[jbcaillau]

@ocots thanks for the update. Val (instead of Index) is fine. I prefer this to having additional fields, possibly empty, such as initial_time_index (would force the dev to check coherence between different fields...)

[PierreMartinon]

Hi guys,
I actually made recent changes in CTDirect about this to get rid of some type instabilities due to the Union for initial.time and final.time. The discretized problem defines 2 sets of variables, one of type Index and one of type Float64 :D

    # initial / final time
    fixed_initial_time::Float64
    fixed_final_time::Float64
    index_initial_time::Index
    index_final_time::Index

with initialization

if is_free_initial_time
     index_initial_time = ocp.initial_time
     fixed_initial_time = 0. # unused
else
     fixed_initial_time = ocp.initial_time
     index_initial_time = Index(1) # unused
end

then the relevant one is used in the getter

function get_final_time(xu, docp)
    if docp.is_free_final_time
        return get_optim_variable(xu, docp)[docp.index_final_time]
    else
        return docp.fixed_final_time
    end
end

Maybe there was a more elegant way to get stable types, but this one seems to work, and I'll just replace Index with Int if you merge this PR :-)

I am currently investigating if I also need to split the Union types for State, Control and Variable. I'll update the memory allocations issue tomorrow.

[ocots]

@ocots thanks for the update. Val (instead of Index) is fine. I prefer this to having additional fields, possibly empty, such as initial_time_index (would force the dev to check coherence between different fields...)

We already have this for instance with lagrange and mayer, with variable when the problem is fixed. We do not have made different types.

About the initial time for instance. The only thing I don't like with the use of the same field for index and time is that the user can think that the returned value for the initial time is a time but it is an index. If we have two fields, this cannot happen.

A possibility is to have only one field but to make a parametric type and use multiple dispatch:

mutable struct OptimalControlModel{
    time_dependence <: TimeDependence,
    variable_dependence <: VariableDependence,
    initial_time <: VariableDependence,
    final_time <: VariableDependence,
}
...
initial_time::Union{Time, Index, Nothing} = nothing
...
end

function initial_time(
    ocp::OptimalControlModel{
        <: TimeDependence, 
        <: VariableDependence, 
        Fixed,
        <: VariableDependence
    }
)
    return ocp.initial_time
end

function initial_time(
    ocp::OptimalControlModel{
        <: TimeDependence, 
        <: VariableDependence, 
        NonFixed,
        <: VariableDependence
    },
    v::Variable
)
    return v[ocp.initial_time]
end

[jbcaillau]

@ocots @PierreMartinon hi! two things:

About the initial time for instance. The only thing I don't like with the use of the same field for index and time is that the user can think that the returned value for the initial time is a time but it is an index. If we have two fields, this cannot happen.

• yes. returning an index that could be interpretated as a value for fixed time would be bad
• in the light of current questions on type instabilities, we should clearly avoid unions

Can we conclude?

[PierreMartinon]

So, to avoid union types, we would have 2 sets of variables, one for fixed times values and one for free times indices ?

For each OCP half the variables would be unused, but the getters would be type stable.

[jbcaillau]

@PierreMartinon Index type will not exist anymore in what @ocots is now refactoring to have stable types in the model. If OK with you, please close the PR.