Thoughts on the C interface

[odow]

@frapac (and other watchers), I'd like to get feedback on potentially removing the C_wrapper part of the C API.

The main reason for removing it is because it is undocumented, and serves as a slightly complicated abstraction layer between MOI and the solver. It introduces a number of ways of doing the same thing:

KNITRO.jl/src/C_wrapper.jl

Lines 375 to 402 in 4db649c

	function KN_add_con_linear_struct(
	m::Model,
	index_cons::Vector{Cint},
	index_vars::Vector{Cint},
	coefs::Vector{Cdouble},
	)
	@assert length(index_cons) == length(index_vars) == length(coefs)
	return KN_add_con_linear_struct(m, length(index_cons), index_cons, index_vars, coefs)
	end
	function KN_add_con_linear_struct(
	m::Model,
	index_con::Integer,
	index_vars::Vector{Cint},
	coefs::Vector{Cdouble},
	)
	@assert length(index_vars) == length(coefs)
	return KN_add_con_linear_struct_one(m, length(index_vars), index_con, index_vars, coefs)
	end
	function KN_add_con_linear_struct(
	m::Model,
	index_con::Integer,
	index_var::Integer,
	coef::Cdouble,
	)
	return KN_add_con_linear_struct_one(m, 1, index_con, [index_var], [coef])
	end

and it uses multiple dispatch to create new functions that look like they're from the C API, but are actually not:

KNITRO.jl/src/C_wrapper.jl

Lines 703 to 725 in 4db649c

	function KN_set_param(m::Model, id::Integer, value::Integer)
	return KN_set_int_param(m, id, value)
	end
	function KN_set_param(m::Model, param::AbstractString, value::Integer)
	return KN_set_int_param_by_name(m, param, value)
	end
	function KN_set_param(m::Model, id::Integer, value::Cdouble)
	return KN_set_double_param(m, id, value)
	end
	function KN_set_param(m::Model, param::AbstractString, value::Cdouble)
	return KN_set_double_param_by_name(m, param, value)
	end
	function KN_set_param(m::Model, id::Integer, value::AbstractString)
	return KN_set_char_param(m, id, value)
	end
	function KN_set_param(m::Model, param::AbstractString, value::AbstractString)
	return KN_set_char_param_by_name(m, param, value)
	end

I've previously removed layers like this from Gurobi/CPLEX/etc and I think it is a big win. There is now only the MOI wrapper, and the upstream documented C API.

The downside, of course, is that there are a large number of examples using the C API, although we could update them to the underlying C:

https://github.com/jump-dev/KNITRO.jl/tree/master/examples

This would of course be horribly breaking, but it'd be nice to decide before releasing KNITRO v1.0.

I guess the other comparison here is Ipopt:

https://github.com/jump-dev/Ipopt.jl/blob/master/src/C_wrapper.jl

It has a very minimal C wrapper, which is mainly just to simplify the @cccallable stuff. So we wouldn't have to throw away the C_wrapper.jl entirely, just the bits that could otherwise be the original C.

[amontoison]

We use the C interfaces of Ipopt.jl and KNITRO.jl for the packages NLPModelsIpopt.jl and NLPModelsKnitro.jl.

[odow]

I guess to clarify, my change would mostly make it so that the only KN_xxx functions are those defined in https://github.com/jump-dev/KNITRO.jl/blob/master/src/libknitro.jl

This would mostly affect calls like this, which, despite calling a KN function are not part of the KNITRO C API.
https://github.com/JuliaSmoothOptimizers/NLPModelsKnitro.jl/blob/1603907741669101cf236af3c6ca11b13ed7c316/src/api.jl#L67-L71

But yes, this would be a breaking change, so we'd need to decide if it was worth it.

My point at the moment is that there is really 3 APIs that we are maintaining: MOI, libknitro.jl, and the in-between kinda-c-and-kinda-julia.

[odow]

Perhaps let me draft a PR, and then we will have something more concrete to discuss.

[odow]

Another problem with the C wrapper is that it just ignores all return codes, so we're not catching errors.

[frapac]

@odow I agree with you: the current C wrapper is a bit clunky, and was developed before Clang.jl has been made broadly available.

In general, I prefer simplicity.

If you want, I can update the examples with the new bare-bone C wrapper.

[odow]

I've made good progress #268. I have't updated the examples yet.

The C_wrapper.jl is now basically only that what is needed to create the callbacks for C and Julia.

[odow]

I'm now much more confidence about the state of the KNITRO C wrapper, and I think simplifying it is a very good idea. The C examples were trivial to update, and in every case, they made things much more explicit.

If we merge this, I'll be responsible for updating NLPModelsKnitro.

[odow]

I updated NLPModelsKnitro: JuliaSmoothOptimizers/NLPModelsKnitro.jl#117, so I think I'm ready to close this issue and release v0.14.

[odow]

Closing because I'm now pretty happy with the C wrapper.

The only changes are around KN_new and KN_free, the license manager, callbacks, KN_solve (because of the multithreading issue) and the novel KN_solution(::Model).

This should be much easier to debug and maintain going forwards.