symforce-org / symforce

Fast symbolic computation, code generation, and nonlinear optimization for robotics
https://symforce.org
Apache License 2.0
1.44k stars 147 forks source link

AssertionError: Symbols in inputs must be unique. Duplicate symbols = [1, 0, 0, 1] #321

Closed VineetTambe closed 1 year ago

VineetTambe commented 1 year ago

I am trying to write codegen for a quadratic cost. The following is the code for it:

q_ref = sf.V3.symbolic("q_ref")

inputs_quad_cost = Values(
    state = sf.V2([q[0], q[1]]).T,
    u = sf.V1(q[2]),
    params = Values(
        Q = sf.Matrix22.eye(),
        R = sf.Scalar(0.0001),
        state_ref = sf.V2([q_ref[0], q_ref[1]]),
        u_ref = sf.V1(q_ref[2]),
    )
)

outputs_quad_cost = Values(
    cost = 0.5 * (state - inputs_quad_cost["params"]["state_ref"]).T * inputs_quad_cost["params"]["Q"] * (state - inputs_quad_cost["params"]["state_ref"]) + 0.5 * (u - inputs_quad_cost["params"]["u_ref"]).T * inputs_quad_cost["params"]["R"] * (u - inputs_quad_cost["params"]["u_ref"])
    )

quad_cost_codegen = codegen.Codegen(
    inputs=inputs_quad_cost,
    outputs=outputs_quad_cost,
    config=codegen.CppConfig(),
    name="quadratic_cost",
    return_key="cost",
)

However, when I define the codegen object I get the following error: AssertionError: Symbols in inputs must be unique. Duplicate symbols = [1, 0, 0, 1]

Here's the stack trace:

---------------------------------------------------------------------------
AssertionError                            Traceback (most recent call last)
Cell In[43], line 1
----> 1 quad_cost_codegen = codegen.Codegen(
      2     inputs=inputs_quad_cost,
      3     outputs=outputs_quad_cost,
      4     config=codegen.CppConfig(),
      5     name="quadratic_cost",
      6     return_key="cost",
      7 )

File [~/symForce/env/lib/python3.10/site-packages/symforce/codegen/codegen.py:208](https://file+.vscode-resource.vscode-cdn.net/home/vrex/symForce/symforce_ws/code-gen/~/symForce/env/lib/python3.10/site-packages/symforce/codegen/codegen.py:208), in Codegen.__init__(self, inputs, outputs, config, name, return_key, sparse_matrices, docstring)
    205 assert all(k.isidentifier() for k in outputs.keys())
    207 # Symbols in inputs must be unique
--> 208 assert len(input_symbols) == len(
    209     input_symbols_list
    210 ), "Symbols in inputs must be unique. Duplicate symbols = {}".format(
    211     [symbol for symbol in input_symbols_list if input_symbols_list.count(symbol) > 1]
    212 )
    214 # Outputs must not have same variable names[/keys](https://file+.vscode-resource.vscode-cdn.net/keys) as inputs
    215 assert all(key not in list(outputs.keys()) for key in inputs.keys())

AssertionError: Symbols in inputs must be unique. Duplicate symbols = [1, 0, 0, 1]

I have narrowed it down to the "Q" that I define in my inputs which is an identity matrix. Can someone help me understand how to resolve the duplicate symbol error?

bradley-solliday-skydio commented 1 year ago

The error message is a bit misleading. That error is supposed to catch issues like this:

x = sf.Symbol("x")
inputs = Values(arg1=x, arg2=x)
outputs = Values(out=2*x)

codegen.Codegen(inputs=inputs, outputs=outputs, config=codegen.CppConfig())

which produces the error

---------------------------------------------------------------------------
AssertionError                            Traceback (most recent call last)
Cell In[12], line 5
      2 inputs = Values(arg1=x, arg2=x)
      3 outputs = Values(out=2*x)
----> 5 codegen.Codegen(inputs=inputs, outputs=outputs, config=codegen.CppConfig())

File ~/symforce/symforce/codegen/codegen.py:208, in Codegen.__init__(self, inputs, outputs, config, name, return_key, sparse_matrices, docstring)
    205 assert all(k.isidentifier() for k in outputs.keys())
    207 # Symbols in inputs must be unique
--> 208 assert len(input_symbols) == len(
    209     input_symbols_list
    210 ), "Symbols in inputs must be unique. Duplicate symbols = {}".format(
    211     [symbol for symbol in input_symbols_list if input_symbols_list.count(symbol) > 1]
    212 )
    214 # Outputs must not have same variable names/keys as inputs
    215 assert all(key not in list(outputs.keys()) for key in inputs.keys())

AssertionError: Symbols in inputs must be unique. Duplicate symbols = [x, x]

What's wrong with the above is that the symbol x appears in both arg1 and arg2 (which is bad as then the Codegen object can't distinguish between the two arguments). (Would also be a problem if the same symbol appeared in two different components of the same argument).

In your example, this error is raised because 1 and 0 appear twice in the input Q (which was a problem as you identified). But this is only a secondary error.

The bigger problem is that Codegen.__init__ expects only symbolic inputs, as each is turned into an argument to the generated function. params.Q and params.R both contain concrete values.

If you want the generated function to have those parameters baked in to the generated function, you can just use them directly in outputs_quad_cost.

Also, personally I find it easier to construct Codegen objects using the Codegen.function class method. For example, you could create your cost function using something that looks like this:

Q = sf.Matrix22.eye()
R = sf.Scalar(0.0001)

def quadratic_cost(state: sf.V2, u: sf.V1, params: Values) -> sf.V1:
    cost = 0.5 * (state - params["state_ref"]).T * Q * (state - params["state_ref"])
    cost += 0.5 * (u - params["u_ref"]).T * R * (u - params["u_ref"])
    return sf.V1(cost)

quad_cost_codegen = codegen.Codegen.function(
    func=quadratic_cost,
    config=codegen.CppConfig(),
    input_types=[sf.V2, sf.V1, Values(state_ref=sf.V2(), u_ref=sf.V1())]
)

Ordinarily, it's not necessary to pass in the input_types argument, but if we want our params input to be a Values rather than broken down into separate arguments, we need to tell Codegen what structure the argument has.

aaron-skydio commented 1 year ago

Closing this as resolved, the underlying issue of not letting you create codegen objects from constants is a duplicate of #170