alphaville / optimization-engine

Nonconvex embedded optimization: code generation for fast real-time optimization + ROS support
https://alphaville.github.io/optimization-engine/
Other
513 stars 53 forks source link

Now with unique names in C based on solver name #51

Closed korken89 closed 5 years ago

korken89 commented 5 years ago

Fixes #46

alphaville commented 5 years ago

@korken89 Just some thoughts on this one: we need to keep in mind that there's the TCP server as well, which needs to call the auto-generated optimizer, so we'll need to rename a few functions there too. Then apart from renaming a few functions, you had mentioned that you intend to generate multiple optimizers; this will call for quite some changes in the Python interface. I just wanted to ask whether you intend to do it in this PR, or you just plan to do just the renaming part. If we have multiple optimizers in the same binary, we will have to rethink entirely how the TCP server will work.

korken89 commented 5 years ago

From the TCP there are no changes, is uses Rust linking, not C, so we should not need to change anything more.

On the extension of the Python, we do not need to do this. I see it as a user will generate multiple crates using the same system so we should not need to change anything there either. I did not mean that the user should be able to generate multiple solvers from one config, let the user have multiple to also have the design decentralized :)

korken89 commented 5 years ago

For the solver and exit status, this I can fix. I was thinking that thought will be the same for all solvers, but it will make it more difficult to generate. I will switch to unique types per solver.

For the typedef, this is only required in C - C++ has a super set of naming rules compared to C :)

I'll fix an example as well, it's in the works.

korken89 commented 5 years ago

I have almost fixed it all for having multiple libs link together, but I am not sure why there are trampoline functions in icasadi? Did you have a specific thought behind those @alphaville ?

The functions such as icasadi_cost_, these should not be needed if I see it correctly.

alphaville commented 5 years ago

@korken89 Functions such as icasadi_cost_ (in icasadi.c) are not needed, strictly speaking. They are there to facilitate the invocation of the corresponding auto-generated functions. Essentially, in lib.rs, we define rust functions such as

pub fn icasadi_cost(u: &[f64], casadi_static_params: &[f64], cost_value: &mut f64) -> c_int {
    unsafe { icasadi_cost_(u.as_ptr(), casadi_static_params.as_ptr(), cost_value) }
}

which call the wrapper (trampoline) function icasadi_cost_, which, in turn, calls CASADI_COST_NAME.

korken89 commented 5 years ago

The icasadi interface has had a complete revamp, any comments on that @alphaville ? I think the interface is ready now.

korken89 commented 5 years ago

Now everything seems to work as it should, we just need to add automated (at least link) testing of the C/C++ bindings

korken89 commented 5 years ago

Testing is implemented, I think this is ready. Comments @alphaville ?

korken89 commented 5 years ago

I pushed most of the changes requested :)

korken89 commented 5 years ago

I added the extra names to the YAML generation

korken89 commented 5 years ago

Yeah, PLACEHOLDER is exactly for that. We could probably just generate it all via Python as well, but my Python-fu is not on top :)