csu-hmc / opty

A library for using direct collocation in the optimization of dynamic systems.
http://opty.readthedocs.io
Other
86 stars 20 forks source link

ufuncify_matrix incompatible with some symbol strings #139

Closed tjstienstra closed 3 months ago

tjstienstra commented 3 months ago

As ufuncify_matrix uses the string representation of each symbol when creating the c code, it actually fails when one uses curly brackets in the string representation. Below is a minimal reproducer.

import numpy as np
import sympy as sm
import sympy.physics.mechanics as me
from opty.direct_collocation import Problem
from opty.utils import create_objective_function

t = me.dynamicsymbols._t
x, v, f = me.dynamicsymbols("x_{1} v f_{ext}")
m, c, k = sm.symbols("m_{pt} c k")
eoms = sm.Matrix([x.diff() - v, m * v.diff() + c * v + k * x - f])
duration = 10
t0, tf = 0.0, duration
N = 100
node_time_interval = duration / (N - 1)

obj, obj_grad = create_objective_function(
    sm.Integral(f**2), (x, v), (f,), (), N, node_time_interval
)
initial_state_constraints = {x: 0, v: 0}
final_state_constraints = {x: 1, v: 0}
instance_constraints = tuple(
    xi.subs({t: t0}) - xi_val for xi, xi_val in initial_state_constraints.items()
) + tuple(xi.subs({t: tf}) - xi_val for xi, xi_val in final_state_constraints.items())

bounds = {x: (-1, 10), v: (-100, 100), f: (-10, 10)}

prob = Problem(
    obj, obj_grad, eoms, (x, v), N, node_time_interval, {m: 1, c: 0.1, k: 1},
    {}, instance_constraints, integration_method="backward euler", bounds=bounds
)
prob.solve(np.zeros(3 * N))

This results in the following ufuncify compile files that fail to compile as they use variables like f_{ext}.

moorepants commented 3 months ago

A quick "fix" is to add in the documentation that symbols should be representable in C via SymPy's printers. This is really a SymPy issue, not opty, right? The CCodePrinter should be made to handle these symbols.

tjstienstra commented 3 months ago

I would indeed expect that it should be solved within sympy, but I still have to check where this should exactly be handled. Lambdify does work, at least for the numpy module.

moorepants commented 3 months ago

This is a related sympy issue: https://github.com/sympy/sympy/issues/7767

moorepants commented 3 months ago

Also related (suggested fix for all printers to handle the curly braces in symbol names). https://github.com/sympy/sympy/issues/23374

Peter230655 commented 3 months ago

A quick "fix" is to add in the documentation that symbols should be representable in C via SymPy's printers. This is really a SymPy issue, not opty, right? The CCodePrinter should be made to handle these symbols. At least for someone like me, totally ignorant of C (or of anything other than Python), this would not have helped me much. Maybe better to list what is not allowed, e.g. curly brackets {} ?

moorepants commented 3 months ago

Maybe better to list what is not allowed, e.g. curly brackets {} ?

It isn't as simple as listing that curly braces are not allowed because there are many characters and even symbol names that are not allowed. The documentation for opty shouldn't have to explain anything about SymPy's C code printers. It is best to fix this in SymPy. We could catch the error and report to the opty user "Something has gone wrong in SymPy C code generation", but that also isn't so simple.

moorepants commented 3 months ago

You can see that we do catch the reserved words in C:

https://github.com/sympy/sympy/blob/d2be7bacd2604e98a642f74028e8f0d7d6084f78/sympy/printing/c.py#L81

moorepants commented 3 months ago

These seem to be the rules in C:

moorepants commented 3 months ago

Also: The length of the variable name can be up to 31 characters.

Peter230655 commented 3 months ago

Clearer now, thanks! Does the sympy C printer 'translate' sympy code into code that C understands, similar to sm.lambdify(..) for python / numpy?

moorepants commented 3 months ago

I opened this: https://github.com/sympy/sympy/pull/26360 as a possible fix.

moorepants commented 3 months ago

Does the sympy C printer 'translate' sympy code into code that C understands, similar to sm.lambdify(..) for python / numpy?

Yes.

Peter230655 commented 3 months ago

Thanks!

moorepants commented 3 months ago

The PR in SymPy was merged, so opty user will get a value error if bad symbols are provided when using SymPy >= 1.13.

Peter230655 commented 3 months ago

So my playing around with opty improved it! :-)

moorepants commented 3 months ago

Yes, I think we've fixed at least three things you've brought up. We just have to figure out how to teach you to fix these things yourself :)

Peter230655 commented 3 months ago

Knowing my (lack of) computer skills, this may well be mission impossible! :-)