From 65e06a38294db5bc3c0d06a6b200faae7660f655 Mon Sep 17 00:00:00 2001 From: Frederik Muthers Date: Thu, 12 Mar 2026 15:24:54 +0100 Subject: [PATCH 01/10] added replacement of subexpressions, extended lookup tables --- agentlib_mpc/data_structures/objective.py | 107 ++++++++++++++++++ agentlib_mpc/models/casadi_model.py | 3 + .../casadi_/core/casadi_backend.py | 10 +- examples/one_room_mpc/physical/simple_mpc.py | 12 ++ 4 files changed, 131 insertions(+), 1 deletion(-) diff --git a/agentlib_mpc/data_structures/objective.py b/agentlib_mpc/data_structures/objective.py index 988ab2c9..b4e2afa5 100644 --- a/agentlib_mpc/data_structures/objective.py +++ b/agentlib_mpc/data_structures/objective.py @@ -6,6 +6,40 @@ from agentlib_mpc.models.casadi_model import CasadiParameter, CasadiInput +def _replace_subexpressions(expr_str): + """ + Replace CasADi subexpression definitions (marked with @N=...) in an expression string + with their actual definitions and inline them into the main expression. + + Args: + expr_str: String representation of a CasADi expression + + Returns: + Expression string with subexpressions inlined + """ + if "@" not in expr_str: + return expr_str + + # Extract all subexpression definitions using regex + defs = dict(re.findall(r"(@\d+)=(.*?),\s*", expr_str)) + + # Remove the definitions from the expression string + clean_expr = re.sub(r"@\d+=.*?,\s*", "", expr_str) + + # Inline subexpressions into the main expression + # Process in order to handle nested references (@2 might reference @1) + for subexpr in sorted(defs.keys(), key=lambda x: int(x[1:])): + clean_expr = clean_expr.replace(subexpr, f"({defs[subexpr]})") + + print("Subexpressions found in objective expression:") + for subexpr, definition in defs.items(): + print(f" {subexpr} = {definition}") + print(f"Expression before removing subexpressions: {expr_str}") + print(f"Cleaned expression after removing subexpressions: {clean_expr}") + + return clean_expr + + class SubObjective: def __init__( self, @@ -64,6 +98,9 @@ def _evaluate_expression(self, expr, df): expr_str = str(expr) + if "@" in expr_str: + expr_str = _replace_subexpressions(expr_str) + # Handle common CasADi functions with simple replacements casadi_replacements = { "sq(": "(", @@ -71,8 +108,26 @@ def _evaluate_expression(self, expr, df): "sqrt(": "sqrt(", "sin(": "sin(", "cos(": "cos(", + "tan(": "tan(", + "asin(": "arcsin(", + "acos(": "arccos(", + "atan(": "arctan(", + "atan2(": "arctan2(", + "sinh(": "sinh(", + "cosh(": "cosh(", + "tanh(": "tanh(", + "asinh(": "arcsinh(", + "acosh(": "arccosh(", + "atanh(": "arctanh(", "exp(": "exp(", "log(": "log(", + "log10(": "log10(", + "pow(": "power(", + "floor(": "floor(", + "ceil(": "ceil(", + "sign(": "sign(", + "fmin(": "minimum(", + "fmax(": "maximum(", } # Apply replacements @@ -93,11 +148,40 @@ def _evaluate_expression(self, expr, df): "sqrt", "sin", "cos", + "tan", + "asin", + "acos", + "atan", + "atan2", + "sinh", + "cosh", + "tanh", + "asinh", + "acosh", + "atanh", "exp", "log", + "log10", "abs", + "pow", + "power", + "floor", + "ceil", + "round", + "sign", + "fmin", + "fmax", "max", "min", + "minimum", + "maximum", + "arcsin", + "arccos", + "arctan", + "arctan2", + "arcsinh", + "arccosh", + "arctanh", ] var_names = re.findall(r"[a-zA-Z][a-zA-Z0-9_]*", expr_str) var_names = [name for name in var_names if name not in casadi_functions] @@ -120,8 +204,27 @@ def _evaluate_expression(self, expr, df): "sqrt": np.sqrt, "sin": np.sin, "cos": np.cos, + "tan": np.tan, + "arcsin": np.arcsin, + "arccos": np.arccos, + "arctan": np.arctan, + "arctan2": np.arctan2, + "sinh": np.sinh, + "cosh": np.cosh, + "tanh": np.tanh, + "arcsinh": np.arcsinh, + "arccosh": np.arccosh, + "arctanh": np.arctanh, "exp": np.exp, "log": np.log, + "log10": np.log10, + "power": np.power, + "floor": np.floor, + "ceil": np.ceil, + "round": np.round, + "sign": np.sign, + "minimum": np.minimum, + "maximum": np.maximum, "max": np.maximum, "min": np.minimum, } @@ -433,6 +536,10 @@ def _evaluate_condition(self, condition, df): Boolean Series with True where condition is true """ condition_str = str(condition) + + # Handle CasADi subexpressions in conditions + if "@" in condition_str: + condition_str = _replace_subexpressions(condition_str) identifier_pattern = r'[a-zA-Z_][a-zA-Z0-9_]*' potential_vars = re.findall(identifier_pattern, condition_str) diff --git a/agentlib_mpc/models/casadi_model.py b/agentlib_mpc/models/casadi_model.py index 6fdb3f9a..13875e63 100644 --- a/agentlib_mpc/models/casadi_model.py +++ b/agentlib_mpc/models/casadi_model.py @@ -336,6 +336,9 @@ def __init__(self, **kwargs): expressions=self.objective, name=str(self.objective) ) ) + # Mark as legacy with a flag (don't override get_casadi_expression, + # as it's needed by the discretization code) + self.objective._is_legacy_wrapped = True warnings.warn( "\033[93mWARNING:\033[0m Model uses the deprecated objective formulation. " "Consider migrating to the new CombinedObjective formulation.\n" diff --git a/agentlib_mpc/optimization_backends/casadi_/core/casadi_backend.py b/agentlib_mpc/optimization_backends/casadi_/core/casadi_backend.py index 53c9c56d..de50a623 100644 --- a/agentlib_mpc/optimization_backends/casadi_/core/casadi_backend.py +++ b/agentlib_mpc/optimization_backends/casadi_/core/casadi_backend.py @@ -290,7 +290,15 @@ def save_result_df( # Calculate objective values df = results.df - objective_names, objective_values = self.approximate_objective(df) + + # Skip objective calculation for legacy objectives (old notation) + # as they may contain conditional expressions that cannot be evaluated + # Legacy objectives are marked with _is_legacy_wrapped flag + if getattr(self.system.objective, '_is_legacy_wrapped', False): + objective_names = [] + objective_values = {} + else: + objective_names, objective_values = self.approximate_objective(df) if not self.results_file_exists(): if not self.config.save_only_stats: diff --git a/examples/one_room_mpc/physical/simple_mpc.py b/examples/one_room_mpc/physical/simple_mpc.py index 686e13fe..49dd3d02 100644 --- a/examples/one_room_mpc/physical/simple_mpc.py +++ b/examples/one_room_mpc/physical/simple_mpc.py @@ -115,6 +115,17 @@ def setup_system(self): name="control_costs", ) + # Malicious definition from Issue #101 + # cooling_load = self.mDot / self.C * (self.T_in - self.T) + # complex_objective = cooling_load **2 - cooling_load / self.load + + # obj1 = self.create_sub_objective( + # expressions=complex_objective, + # weight=self.r_mDot, + # name="control_costs", + # ) + + obj2 = self.create_sub_objective( expressions=self.T_slack**2, weight=1000, name="temp_slack" ) @@ -129,6 +140,7 @@ def setup_system(self): # ] # ) + # Also supported: # return obj1 + obj2 # return obj1 + obj2 * 0.5 From 9b02f6d7b7cdc1199f1ad085f63e120dac00fba9 Mon Sep 17 00:00:00 2001 From: Frederik Muthers Date: Thu, 26 Mar 2026 14:01:35 +0100 Subject: [PATCH 02/10] added support for logical casadi operations --- agentlib_mpc/data_structures/objective.py | 89 ++++- .../physical/simple_mpc_objective_testing.py | 351 ++++++++++++++++++ 2 files changed, 439 insertions(+), 1 deletion(-) create mode 100644 examples/one_room_mpc/physical/simple_mpc_objective_testing.py diff --git a/agentlib_mpc/data_structures/objective.py b/agentlib_mpc/data_structures/objective.py index b4e2afa5..6e84a375 100644 --- a/agentlib_mpc/data_structures/objective.py +++ b/agentlib_mpc/data_structures/objective.py @@ -40,6 +40,58 @@ def _replace_subexpressions(expr_str): return clean_expr +def _replace_ternary(eval_str): + """ + Replace CasADi ternary expressions like (cond)?true:false with where(cond, true, false). + """ + while "?" in eval_str: + q_idx = eval_str.rfind("?") + c_idx = eval_str.find(":", q_idx) + if q_idx == -1 or c_idx == -1: + break + + # Backward search for condition + pares = 0 + cond_start = 0 + for i in range(q_idx - 1, -1, -1): + if eval_str[i] == ")": + pares += 1 + elif eval_str[i] == "(": + pares -= 1 + if pares < 0: + cond_start = i + 1 + break + else: + cond_start = 0 + + # Forward search for false_val + pares = 0 + false_end = len(eval_str) + for i in range(c_idx + 1, len(eval_str)): + if eval_str[i] == "(": + pares += 1 + elif eval_str[i] == ")": + pares -= 1 + if pares < 0: + false_end = i + break + elif pares == 0 and eval_str[i] in "+-*/,@": + if i > c_idx + 1: + false_end = i + break + + cond_str = eval_str[cond_start:q_idx] + true_str = eval_str[q_idx + 1 : c_idx] + false_str = eval_str[c_idx + 1 : false_end] + + eval_str = ( + eval_str[:cond_start] + + f"where({cond_str}, {true_str}, {false_str})" + + eval_str[false_end:] + ) + return eval_str + + class SubObjective: def __init__( self, @@ -92,6 +144,11 @@ def _evaluate_expression(self, expr, df): direct expression with a casadi function and map from the available variables""" # Handle simple named variables first var_name = expr.name + + # Debug + print(f"Evaluating expression: {expr}") + + for col_type in ["variable", "parameter"]: if (col_type, var_name) in df.columns: return df.loc[:, (col_type, var_name)].values[:-1] @@ -101,6 +158,12 @@ def _evaluate_expression(self, expr, df): if "@" in expr_str: expr_str = _replace_subexpressions(expr_str) + if "?" in expr_str: + expr_str = _replace_ternary(expr_str) + + # Replace logical NOT '!' with bitwise NOT '~' for NumPy arrays + expr_str = re.sub(r"(?=!])!(?!=)", "~", expr_str) + # Handle common CasADi functions with simple replacements casadi_replacements = { "sq(": "(", @@ -182,6 +245,7 @@ def _evaluate_expression(self, expr, df): "arcsinh", "arccosh", "arctanh", + "where", ] var_names = re.findall(r"[a-zA-Z][a-zA-Z0-9_]*", expr_str) var_names = [name for name in var_names if name not in casadi_functions] @@ -227,6 +291,7 @@ def _evaluate_expression(self, expr, df): "maximum": np.maximum, "max": np.maximum, "min": np.minimum, + "where": np.where, } ) @@ -536,11 +601,22 @@ def _evaluate_condition(self, condition, df): Boolean Series with True where condition is true """ condition_str = str(condition) + print(f"Evaluating condition (raw CasADi): {condition_str}") # Handle CasADi subexpressions in conditions if "@" in condition_str: condition_str = _replace_subexpressions(condition_str) + # Handle CasADi ternary expressions (if_else) + if "?" in condition_str: + condition_str = _replace_ternary(condition_str) + + # Replace CasADi logical operators with Python equivalents for eval + condition_str = condition_str.replace("&&", " and ") + condition_str = condition_str.replace("||", " or ") + condition_str = re.sub(r"(?=!])!(?!=)", " not ", condition_str) + print(f"Evaluating condition (translated for eval): {condition_str}") + identifier_pattern = r'[a-zA-Z_][a-zA-Z0-9_]*' potential_vars = re.findall(identifier_pattern, condition_str) @@ -570,7 +646,9 @@ def _evaluate_condition(self, condition, df): local_vars[var_name] = values[i] try: eval_str = condition_str - result = eval(eval_str, {"__builtins__": {}, "abs": abs, "min": min, "max": max}, local_vars) + # Include standard math/numpy functions required by the parser + safe_dict = {"__builtins__": {}, "abs": abs, "min": min, "max": max, "where": np.where} + result = eval(eval_str, safe_dict, local_vars) mask[i] = bool(result) except Exception as e: @@ -578,3 +656,12 @@ def _evaluate_condition(self, condition, df): mask[i] = False return pd.Series(mask, index=df.index) + + +if __name__ == "__main__": + + # test ternary replacement + test_expr = "(x > 0) ? (y + 1) : (y - 1)" + replaced_expr = _replace_ternary(test_expr) + print(f"Original expression: {test_expr}") + print(f"Replaced expression: {replaced_expr}") \ No newline at end of file diff --git a/examples/one_room_mpc/physical/simple_mpc_objective_testing.py b/examples/one_room_mpc/physical/simple_mpc_objective_testing.py new file mode 100644 index 00000000..c23b03e3 --- /dev/null +++ b/examples/one_room_mpc/physical/simple_mpc_objective_testing.py @@ -0,0 +1,351 @@ +import logging +import os +from pathlib import Path +from typing import List +import casadi as ca + +import pandas as pd + +from agentlib_mpc.models.casadi_model import ( + CasadiModel, + CasadiInput, + CasadiState, + CasadiParameter, + CasadiOutput, + CasadiModelConfig, +) +from agentlib.utils.multi_agent_system import LocalMASAgency +from agentlib_mpc.utils.plotting.interactive import show_dashboard + +logger = logging.getLogger(__name__) + +# script variables +ub = 295.15 +prediction_horizon = 300*15 +switch_time = 600 + +class MyCasadiModelConfig(CasadiModelConfig): + inputs: List[CasadiInput] = [ + # controls + CasadiInput( + name="mDot", + value=0.0225, + unit="m³/s", + description="Air mass flow into zone", + ), + # disturbances + CasadiInput( + name="load", value=150, unit="W", description="Heat load into zone" + ), + CasadiInput( + name="T_in", value=290.15, unit="K", description="Inflow air temperature" + ), + # settings + CasadiInput( + name="T_upper", + value=294.15, + unit="K", + description="Upper boundary (soft) for T.", + ), + ] + + states: List[CasadiState] = [ + # differential + CasadiState( + name="T", value=293.15, unit="K", description="Temperature of zone" + ), + # algebraic + # slack variables + CasadiState( + name="T_slack", + value=0, + unit="K", + description="Slack variable of temperature of zone", + ), + ] + + parameters: List[CasadiParameter] = [ + CasadiParameter( + name="cp", + value=1000, + unit="J/kg*K", + description="thermal capacity of the air", + ), + CasadiParameter( + name="C", value=100000, unit="J/K", description="thermal capacity of zone" + ), + CasadiParameter( + name="s_T", + value=1, + unit="-", + description="Weight for T in constraint function", + ), + CasadiParameter( + name="r_mDot", + value=1, + unit="-", + description="Weight for mDot in objective function", + ), + CasadiParameter( + name="r_mDot2", + value=1, + unit="-", + description="Weight for mDot in objective function", + ), + CasadiParameter( + name="switch", + value=600, + unit="s", + description="Time at which the objective is changed" + ) + ] + outputs: List[CasadiOutput] = [ + CasadiOutput(name="T_out", unit="K", description="Temperature of zone"), + CasadiOutput(name="switch_test", unit="-", description="Test variable to check the change of the objective") + ] + + +class MyCasadiModel(CasadiModel): + config: MyCasadiModelConfig + + def setup_system(self): + # Define ode + + self.T.ode = ( + self.cp * self.mDot / self.C * (self.T_in - self.T) + self.load / self.C + ) + + # Define ae + self.T_out.alg = self.T # math operation to get the symbolic variable + + # Constraints: List[(lower bound, function, upper bound)] + self.constraints = [ + # soft constraints + (0, self.T + self.T_slack, self.T_upper), + ] + + # Objective function + # Objective1 when time= switch) + obj2_mDot = self.create_sub_objective( + expressions=self.mDot, + weight= self.r_mDot2, + name="mDot_cost_doubled" + ) + obj2_slack = self.create_sub_objective( + expressions=self.T_slack**2, + name="temperature_slack_2" + ) + objective2_1 = self.create_combined_objective(obj2_mDot) + objective2_2 = self.create_combined_objective(obj2_slack) + + objective2 = objective2_1 + objective2_2*self.s_T + + # Conditional objective based on time + condition = self.time < self.switch.sym + + condition = self.time <= self.switch.sym + condition = self.time >= self.switch.sym + condition = self.time == self.switch.sym + condition = self.time != self.switch.sym + condition = ca.logic_and(self.time < self.switch.sym, self.time >= 0) + condition = ca.logic_or(self.time < self.switch.sym, self.time >= 7200) + condition = ca.logic_not(self.time < self.switch.sym) + condition = ca.if_else(self.time < self.switch.sym, 1, 0) > 0 + + condition = ca.if_else(self.time < self.switch.sym, ca.if_else(self.time > 100, 1, ca.if_else(self.time == 0, 0, 1)), ca.if_else(self.time < 300, 0, 0)) > 0 + + + objective = self.create_conditional_objective( + (condition, objective1), + default_objective=objective2 + ) + + # Test extreme nesting for evaluate_expression (math stream) + self.switch_test.alg = ca.if_else( + self.time < self.switch.sym, + ca.if_else( + self.time < 300, + ca.if_else(self.time == 0, 1, 2), + 3 + ), + ca.if_else( + self.time < 1000, + 4, + 5 + ) + ) + + return objective + + +ENV_CONFIG = {"rt": False, "factor": 0.01, "t_sample": 60} + +AGENT_MPC = { + "id": "myMPCAgent", + "modules": [ + {"module_id": "Ag1Com", "type": "local_broadcast"}, + { + "module_id": "myMPC", + "type": "agentlib_mpc.mpc", + "optimization_backend": { + "type": "casadi", + "model": {"type": {"file": __file__, "class_name": "MyCasadiModel"}}, + "discretization_options": { + "method": "multiple_shooting" + }, + "solver": { + "name": "ipopt", # use fatrop with casadi 3.6.6 for speedup + }, + "results_file": "results//mpc.csv", + "save_results": True, + "overwrite_result_file": True, + }, + "time_step": 300, + "prediction_horizon": 15, + "parameters": [ + {"name": "s_T", "value": 3}, + {"name": "r_mDot", "value": 1}, + {"name": "r_mDot2", "value": 5}, + {"name": "switch", "value": 600}, + ], + "inputs": [ + {"name": "T_in", "value": 290.15}, + {"name": "load", "value": 150}, + {"name": "T_upper", "value": ub}, + ], + "controls": [{"name": "mDot", "value": 0.02, "ub": 0.05, "lb": 0}], + "outputs": [{"name": "T_out"}, + {"name": "switch_test"} + ], + "states": [ + { + "name": "T", + "value": 298.16, + "ub": 303.15, + "lb": 288.15, + "alias": "T", + "source": "SimAgent", + }, + + ], + }, + ], +} +AGENT_SIM = { + "id": "SimAgent", + "modules": [ + {"module_id": "Ag1Com", "type": "local_broadcast"}, + { + "module_id": "room", + "type": "simulator", + "model": { + "type": {"file": __file__, "class_name": "MyCasadiModel"}, + "states": [{"name": "T", "value": 298.16}], + }, + "t_sample": 10, + "update_inputs_on_callback": False, + "save_results": True, + "outputs": [ + {"name": "T_out", "value": 298, "alias": "T"}, + ], + "inputs": [ + {"name": "mDot", "value": 0.02, "alias": "mDot"}, + ], + }, + ], +} + + +def run_example( + with_plots=True, log_level=logging.INFO, until=10000, with_dashboard=False +): + # Change the working directly so that relative paths work + os.chdir(Path(__file__).parent) + + # Set the log-level + logging.basicConfig(level=log_level) + mas = LocalMASAgency( + agent_configs=[AGENT_MPC, AGENT_SIM], env=ENV_CONFIG, variable_logging=False + ) + mas.run(until=until) + results = mas.get_results(cleanup=False) + mpc_results = results["myMPCAgent"]["myMPC"] + sim_res = results["SimAgent"]["room"] + + if with_dashboard: + if with_dashboard: + from agentlib_mpc.utils.analysis import load_mpc_stats + + mpc_result_file = "results//mpc.csv" + + try: + stats = load_mpc_stats(mpc_result_file) + except Exception: + stats = None + + show_dashboard(mpc_results, stats) + + if with_plots: + plot(mpc_results, sim_res, until) + + return results + + +def plot(mpc_results: pd.DataFrame, sim_res: pd.DataFrame, until: float): + import matplotlib.pyplot as plt + from agentlib_mpc.utils.plotting.mpc import plot_mpc + + fig, ax = plt.subplots(2, 1, sharex=True) + t_sim = sim_res["T_out"] + t_sample = t_sim.index[1] - t_sim.index[0] + aie_kh = (t_sim - ub).abs().sum() * t_sample / 3600 + energy_cost_kWh = ( + (sim_res["mDot"] * (sim_res["T_out"] - sim_res["T_in"])).sum() + * t_sample + * 1 + / 3600 + ) # cp is 1 + print(f"Absoulute integral error: {aie_kh} Kh.") + print(f"Cooling energy used: {energy_cost_kWh} kWh.") + + plot_mpc( + series=mpc_results["variable"]["T"] - 273.15, + ax=ax[0], + plot_actual_values=True, + plot_predictions=True, + ) + ax[0].axhline(ub - 273.15, color="grey", linestyle="--", label="upper boundary") + plot_mpc( + series=mpc_results["variable"]["mDot"], + ax=ax[1], + plot_actual_values=True, + plot_predictions=True, + ) + + ax[1].legend() + ax[0].legend() + ax[0].set_ylabel("$T_{room}$ / °C") + ax[1].set_ylabel("$\dot{m}_{air}$ / kg/s") + ax[1].set_xlabel("simulation time / s") + ax[1].set_ylim([0, 0.06]) + ax[1].set_xlim([0, until]) + plt.show() + + +if __name__ == "__main__": + run_example( + with_plots=True, with_dashboard=True, until=7200, log_level=logging.INFO + ) From df45e7646e07d87eedb32146377710ebee278f63 Mon Sep 17 00:00:00 2001 From: Frederik Muthers Date: Thu, 16 Apr 2026 16:52:43 +0200 Subject: [PATCH 03/10] cleanup objective.py --- agentlib_mpc/data_structures/objective.py | 76 ++++--------------- .../physical/simple_mpc_objective_testing.py | 10 ++- 2 files changed, 22 insertions(+), 64 deletions(-) diff --git a/agentlib_mpc/data_structures/objective.py b/agentlib_mpc/data_structures/objective.py index 6e84a375..3f48337f 100644 --- a/agentlib_mpc/data_structures/objective.py +++ b/agentlib_mpc/data_structures/objective.py @@ -6,16 +6,11 @@ from agentlib_mpc.models.casadi_model import CasadiParameter, CasadiInput -def _replace_subexpressions(expr_str): +def _replace_subexpressions(expr_str, verbose=False): """ Replace CasADi subexpression definitions (marked with @N=...) in an expression string with their actual definitions and inline them into the main expression. - - Args: - expr_str: String representation of a CasADi expression - - Returns: - Expression string with subexpressions inlined + """ if "@" not in expr_str: return expr_str @@ -31,11 +26,12 @@ def _replace_subexpressions(expr_str): for subexpr in sorted(defs.keys(), key=lambda x: int(x[1:])): clean_expr = clean_expr.replace(subexpr, f"({defs[subexpr]})") - print("Subexpressions found in objective expression:") - for subexpr, definition in defs.items(): - print(f" {subexpr} = {definition}") - print(f"Expression before removing subexpressions: {expr_str}") - print(f"Cleaned expression after removing subexpressions: {clean_expr}") + if verbose: + print("Subexpressions found in objective expression:") + for subexpr, definition in defs.items(): + print(f" {subexpr} = {definition}") + print(f"Expression before removing subexpressions: {expr_str}") + print(f"Cleaned expression after removing subexpressions: {clean_expr}") return clean_expr @@ -205,48 +201,11 @@ def _evaluate_expression(self, expr, df): eval_str = eval_str.replace(casadi_func, replacement) # Extract variable names, filtering out CasADi function names - casadi_functions = [ - "sq", - "fabs", - "sqrt", - "sin", - "cos", - "tan", - "asin", - "acos", - "atan", - "atan2", - "sinh", - "cosh", - "tanh", - "asinh", - "acosh", - "atanh", - "exp", - "log", - "log10", - "abs", - "pow", - "power", - "floor", - "ceil", - "round", - "sign", - "fmin", - "fmax", - "max", - "min", - "minimum", - "maximum", - "arcsin", - "arccos", - "arctan", - "arctan2", - "arcsinh", - "arccosh", - "arctanh", - "where", - ] + base_funcs = [k.rstrip("(") for k in casadi_replacements.keys()] + replaced_funcs = [v.rstrip("(") for v in casadi_replacements.values() if v.endswith("(")] + extra_funcs = ["round", "max", "min", "minimum", "maximum", "where", "abs", "power"] + casadi_functions = list(set(base_funcs + replaced_funcs + extra_funcs)) + var_names = re.findall(r"[a-zA-Z][a-zA-Z0-9_]*", expr_str) var_names = [name for name in var_names if name not in casadi_functions] @@ -656,12 +615,3 @@ def _evaluate_condition(self, condition, df): mask[i] = False return pd.Series(mask, index=df.index) - - -if __name__ == "__main__": - - # test ternary replacement - test_expr = "(x > 0) ? (y + 1) : (y - 1)" - replaced_expr = _replace_ternary(test_expr) - print(f"Original expression: {test_expr}") - print(f"Replaced expression: {replaced_expr}") \ No newline at end of file diff --git a/examples/one_room_mpc/physical/simple_mpc_objective_testing.py b/examples/one_room_mpc/physical/simple_mpc_objective_testing.py index c23b03e3..3ede408c 100644 --- a/examples/one_room_mpc/physical/simple_mpc_objective_testing.py +++ b/examples/one_room_mpc/physical/simple_mpc_objective_testing.py @@ -136,7 +136,15 @@ def setup_system(self): weight=self.s_T, name="temperature_slack" ) - objective1 = self.create_combined_objective(obj1_mDot, obj1_slack, normalization=10) + objective1_base = self.create_combined_objective(obj1_mDot, obj1_slack, normalization=10) + + inner_condition = ca.logic_and(self.mDot.sym > 0.01, self.T.sym > 293.0) + objective1_alternative = self.create_combined_objective(obj1_mDot, normalization=1) + + objective1 = self.create_conditional_objective( + (inner_condition, objective1_alternative), + default_objective=objective1_base + ) # Objective 2 (when time >= switch) obj2_mDot = self.create_sub_objective( From ff82e9ebb7912d7e6b0e73210ffb0c6ffb153392 Mon Sep 17 00:00:00 2001 From: Frederik Muthers Date: Thu, 16 Apr 2026 17:36:54 +0200 Subject: [PATCH 04/10] cleanup comments --- agentlib_mpc/data_structures/objective.py | 12 +----------- agentlib_mpc/models/casadi_model.py | 3 +-- .../casadi_/core/casadi_backend.py | 4 +--- examples/one_room_mpc/physical/simple_mpc.py | 12 ------------ 4 files changed, 3 insertions(+), 28 deletions(-) diff --git a/agentlib_mpc/data_structures/objective.py b/agentlib_mpc/data_structures/objective.py index 3f48337f..4a638af5 100644 --- a/agentlib_mpc/data_structures/objective.py +++ b/agentlib_mpc/data_structures/objective.py @@ -141,10 +141,6 @@ def _evaluate_expression(self, expr, df): # Handle simple named variables first var_name = expr.name - # Debug - print(f"Evaluating expression: {expr}") - - for col_type in ["variable", "parameter"]: if (col_type, var_name) in df.columns: return df.loc[:, (col_type, var_name)].values[:-1] @@ -560,13 +556,10 @@ def _evaluate_condition(self, condition, df): Boolean Series with True where condition is true """ condition_str = str(condition) - print(f"Evaluating condition (raw CasADi): {condition_str}") - # Handle CasADi subexpressions in conditions if "@" in condition_str: condition_str = _replace_subexpressions(condition_str) - # Handle CasADi ternary expressions (if_else) if "?" in condition_str: condition_str = _replace_ternary(condition_str) @@ -574,7 +567,6 @@ def _evaluate_condition(self, condition, df): condition_str = condition_str.replace("&&", " and ") condition_str = condition_str.replace("||", " or ") condition_str = re.sub(r"(?=!])!(?!=)", " not ", condition_str) - print(f"Evaluating condition (translated for eval): {condition_str}") identifier_pattern = r'[a-zA-Z_][a-zA-Z0-9_]*' potential_vars = re.findall(identifier_pattern, condition_str) @@ -605,9 +597,7 @@ def _evaluate_condition(self, condition, df): local_vars[var_name] = values[i] try: eval_str = condition_str - # Include standard math/numpy functions required by the parser - safe_dict = {"__builtins__": {}, "abs": abs, "min": min, "max": max, "where": np.where} - result = eval(eval_str, safe_dict, local_vars) + result = eval(eval_str, {"__builtins__": {}, "abs": abs, "min": min, "max": max, "where": np.where}, local_vars) mask[i] = bool(result) except Exception as e: diff --git a/agentlib_mpc/models/casadi_model.py b/agentlib_mpc/models/casadi_model.py index 13875e63..db3c4bc0 100644 --- a/agentlib_mpc/models/casadi_model.py +++ b/agentlib_mpc/models/casadi_model.py @@ -336,8 +336,7 @@ def __init__(self, **kwargs): expressions=self.objective, name=str(self.objective) ) ) - # Mark as legacy with a flag (don't override get_casadi_expression, - # as it's needed by the discretization code) + # Mark as legacy with flag self.objective._is_legacy_wrapped = True warnings.warn( "\033[93mWARNING:\033[0m Model uses the deprecated objective formulation. " diff --git a/agentlib_mpc/optimization_backends/casadi_/core/casadi_backend.py b/agentlib_mpc/optimization_backends/casadi_/core/casadi_backend.py index de50a623..5de5797f 100644 --- a/agentlib_mpc/optimization_backends/casadi_/core/casadi_backend.py +++ b/agentlib_mpc/optimization_backends/casadi_/core/casadi_backend.py @@ -291,9 +291,7 @@ def save_result_df( # Calculate objective values df = results.df - # Skip objective calculation for legacy objectives (old notation) - # as they may contain conditional expressions that cannot be evaluated - # Legacy objectives are marked with _is_legacy_wrapped flag + # Skip objective calculation for legacy objectives if getattr(self.system.objective, '_is_legacy_wrapped', False): objective_names = [] objective_values = {} diff --git a/examples/one_room_mpc/physical/simple_mpc.py b/examples/one_room_mpc/physical/simple_mpc.py index 49dd3d02..686e13fe 100644 --- a/examples/one_room_mpc/physical/simple_mpc.py +++ b/examples/one_room_mpc/physical/simple_mpc.py @@ -115,17 +115,6 @@ def setup_system(self): name="control_costs", ) - # Malicious definition from Issue #101 - # cooling_load = self.mDot / self.C * (self.T_in - self.T) - # complex_objective = cooling_load **2 - cooling_load / self.load - - # obj1 = self.create_sub_objective( - # expressions=complex_objective, - # weight=self.r_mDot, - # name="control_costs", - # ) - - obj2 = self.create_sub_objective( expressions=self.T_slack**2, weight=1000, name="temp_slack" ) @@ -140,7 +129,6 @@ def setup_system(self): # ] # ) - # Also supported: # return obj1 + obj2 # return obj1 + obj2 * 0.5 From abba4c07a110d6656d9de82f59928f647563688b Mon Sep 17 00:00:00 2001 From: sarahleidolf Date: Thu, 13 Aug 2026 13:42:23 +0200 Subject: [PATCH 05/10] fix merge bug --- agentlib_mpc/data_structures/objective.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/agentlib_mpc/data_structures/objective.py b/agentlib_mpc/data_structures/objective.py index ef1ca151..212b9e1a 100644 --- a/agentlib_mpc/data_structures/objective.py +++ b/agentlib_mpc/data_structures/objective.py @@ -149,7 +149,9 @@ def _replace_ternary(eval_str): + f"where({cond_str}, {true_str}, {false_str})" + eval_str[false_end:] ) - return eval_strclass SubObjective: + return eval_str + +class SubObjective: _warned_names = set() @@ -683,10 +685,11 @@ def _evaluate_condition(self, condition, df): Boolean Series with True where condition is true """ condition_str = str(condition) - + if "@" in condition_str: condition_str = _replace_subexpressions(condition_str) - if "?" in condition_str: + + if "?" in condition_str: condition_str = _replace_ternary(condition_str) # Replace CasADi logical operators with Python equivalents for eval From 1a21d72ad5adb759ecbbdf9a84c58a40495052a8 Mon Sep 17 00:00:00 2001 From: sarahleidolf Date: Thu, 13 Aug 2026 15:56:03 +0200 Subject: [PATCH 06/10] minor clean up --- CHANGELOG.md | 3 ++ agentlib_mpc/__init__.py | 2 +- .../physical/simple_mpc_objective_testing.py | 30 ++++++------------- 3 files changed, 13 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e26bcd32..710eda42 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,8 @@ # Changelog +## 1.04 +- improve objective logging + ## 1.0.3 - Bug fix: circular import of objective in casadi model diff --git a/agentlib_mpc/__init__.py b/agentlib_mpc/__init__.py index 8f225c1b..4089fb0e 100644 --- a/agentlib_mpc/__init__.py +++ b/agentlib_mpc/__init__.py @@ -4,4 +4,4 @@ from .modules import MODULE_TYPES from .models import MODEL_TYPES -__version__ = "1.0.3" +__version__ = "1.0.4" diff --git a/examples/one_room_mpc/physical/simple_mpc_objective_testing.py b/examples/one_room_mpc/physical/simple_mpc_objective_testing.py index 3ede408c..5cd75f71 100644 --- a/examples/one_room_mpc/physical/simple_mpc_objective_testing.py +++ b/examples/one_room_mpc/physical/simple_mpc_objective_testing.py @@ -163,18 +163,6 @@ def setup_system(self): # Conditional objective based on time condition = self.time < self.switch.sym - - condition = self.time <= self.switch.sym - condition = self.time >= self.switch.sym - condition = self.time == self.switch.sym - condition = self.time != self.switch.sym - condition = ca.logic_and(self.time < self.switch.sym, self.time >= 0) - condition = ca.logic_or(self.time < self.switch.sym, self.time >= 7200) - condition = ca.logic_not(self.time < self.switch.sym) - condition = ca.if_else(self.time < self.switch.sym, 1, 0) > 0 - - condition = ca.if_else(self.time < self.switch.sym, ca.if_else(self.time > 100, 1, ca.if_else(self.time == 0, 0, 1)), ca.if_else(self.time < 300, 0, 0)) > 0 - objective = self.create_conditional_objective( (condition, objective1), @@ -293,18 +281,18 @@ def run_example( mpc_results = results["myMPCAgent"]["myMPC"] sim_res = results["SimAgent"]["room"] + if with_dashboard: - if with_dashboard: - from agentlib_mpc.utils.analysis import load_mpc_stats + from agentlib_mpc.utils.analysis import load_mpc_stats - mpc_result_file = "results//mpc.csv" + mpc_result_file = "results//mpc.csv" - try: - stats = load_mpc_stats(mpc_result_file) - except Exception: - stats = None + try: + stats = load_mpc_stats(mpc_result_file) + except Exception: + stats = None - show_dashboard(mpc_results, stats) + show_dashboard(mpc_results, stats) if with_plots: plot(mpc_results, sim_res, until) @@ -326,7 +314,7 @@ def plot(mpc_results: pd.DataFrame, sim_res: pd.DataFrame, until: float): * 1 / 3600 ) # cp is 1 - print(f"Absoulute integral error: {aie_kh} Kh.") + print(f"Absolute integral error: {aie_kh} Kh.") print(f"Cooling energy used: {energy_cost_kWh} kWh.") plot_mpc( From 3bc818e1cbfbfb14393e21c6516581117347f2ca Mon Sep 17 00:00:00 2001 From: Frederik Muthers Date: Mon, 31 Aug 2026 15:19:49 +0200 Subject: [PATCH 07/10] fixed regex & square handling, added ci test --- agentlib_mpc/data_structures/objective.py | 96 ++++++++--- .../physical/simple_mpc_objective_testing.py | 15 +- tests/test_objective.py | 158 ++++++++++++++++++ 3 files changed, 249 insertions(+), 20 deletions(-) create mode 100644 tests/test_objective.py diff --git a/agentlib_mpc/data_structures/objective.py b/agentlib_mpc/data_structures/objective.py index 212b9e1a..c2f3179d 100644 --- a/agentlib_mpc/data_structures/objective.py +++ b/agentlib_mpc/data_structures/objective.py @@ -70,6 +70,31 @@ def evaluate(self, df): return result +def _split_top_level(s, sep=","): + """ + Split a string on `sep`, but only at nesting depth 0 (i.e. not inside + parentheses or brackets). Used to parse CasADi's printed expressions, + where subexpression definitions are comma-separated at the top level + but may themselves contain commas (e.g. function calls like + `if_else(a, b, c)`). + """ + parts = [] + depth = 0 + current = [] + for ch in s: + if ch in "([": + depth += 1 + elif ch in ")]": + depth -= 1 + if ch == sep and depth == 0: + parts.append("".join(current)) + current = [] + else: + current.append(ch) + parts.append("".join(current)) + return parts + + def _replace_subexpressions(expr_str, verbose=False): """ Replace CasADi subexpression definitions (marked with @N=...) in an expression string @@ -78,18 +103,35 @@ def _replace_subexpressions(expr_str, verbose=False): """ if "@" not in expr_str: return expr_str - - # Extract all subexpression definitions using regex - defs = dict(re.findall(r"(@\d+)=(.*?),\s*", expr_str)) - - # Remove the definitions from the expression string - clean_expr = re.sub(r"@\d+=.*?,\s*", "", expr_str) - + + # Split into top-level comma-separated segments: each is either a + # `@N=` or (for the last segment) the main expression. + # Splitting at top level only (not inside parentheses) ensures + # definitions containing commas, e.g. if_else(a, b, c), stay intact. + parts = _split_top_level(expr_str) + + defs = {} + remainder_parts = [] + for part in parts: + part = part.strip() + match = re.match(r"^(@\d+)=(.*)$", part, re.DOTALL) + if match: + defs[match.group(1)] = match.group(2) + else: + remainder_parts.append(part) + + clean_expr = ", ".join(remainder_parts) + # Inline subexpressions into the main expression - # Process in order to handle nested references (@2 might reference @1) + # Process in order to handle nested references (@2 might reference @1), + # substituting into both the main expression and any remaining + # (not-yet-resolved) definitions. for subexpr in sorted(defs.keys(), key=lambda x: int(x[1:])): - clean_expr = clean_expr.replace(subexpr, f"({defs[subexpr]})") - + definition = f"({defs[subexpr]})" + clean_expr = clean_expr.replace(subexpr, definition) + for other in defs: + defs[other] = defs[other].replace(subexpr, definition) + if verbose: print("Subexpressions found in objective expression:") for subexpr, definition in defs.items(): @@ -151,6 +193,30 @@ def _replace_ternary(eval_str): ) return eval_str +def _replace_sq_calls(expr_str): + """ + Replace CasADi's sq(x) (square) calls with (x)**2, matching each call's + own closing parenthesis rather than assuming sq(...) spans the whole + expression - so terms like r*x + s*sq(y) only square the y term. + """ + while True: + idx = expr_str.find("sq(") + if idx == -1: + break + start = idx + len("sq(") + depth = 1 + i = start + while depth > 0: + if expr_str[i] == "(": + depth += 1 + elif expr_str[i] == ")": + depth -= 1 + i += 1 + inner = expr_str[start : i - 1] + expr_str = expr_str[:idx] + f"({inner})**2" + expr_str[i:] + return expr_str + + class SubObjective: _warned_names = set() @@ -271,11 +337,7 @@ def _evaluate_expression(self, expr, df): } # Apply replacements - eval_str = expr_str - is_square = False - if "sq(" in eval_str: - is_square = True - eval_str = eval_str.replace("sq(", "(") + eval_str = _replace_sq_calls(expr_str) for casadi_func, replacement in casadi_replacements.items(): if casadi_func != "sq(": # already handled above @@ -340,10 +402,6 @@ def _evaluate_expression(self, expr, df): result = eval(eval_str, {"__builtins__": {}}, safe_dict) - # Apply square if it was sq() function - if is_square: - result = result**2 - return result except SyntaxError as e: diff --git a/examples/one_room_mpc/physical/simple_mpc_objective_testing.py b/examples/one_room_mpc/physical/simple_mpc_objective_testing.py index 5cd75f71..e58757c0 100644 --- a/examples/one_room_mpc/physical/simple_mpc_objective_testing.py +++ b/examples/one_room_mpc/physical/simple_mpc_objective_testing.py @@ -156,10 +156,23 @@ def setup_system(self): expressions=self.T_slack**2, name="temperature_slack_2" ) + + # Regression test for _replace_subexpressions: reusing the same fmin() + # node forces CasADi to factor it into a shared subexpression, printed + # as "@1=fmin(mDot,T_slack), (@1+sq(@1))". The comma inside fmin(...) + # must not be mistaken for the end of the @1 definition. + shared_min = ca.fmin(self.mDot.sym, self.T_slack.sym) + obj2_comma_regression = self.create_sub_objective( + expressions=shared_min + shared_min**2, + weight=1, + name="comma_in_subexpression_regression" + ) + objective2_1 = self.create_combined_objective(obj2_mDot) objective2_2 = self.create_combined_objective(obj2_slack) + objective2_3 = self.create_combined_objective(obj2_comma_regression) - objective2 = objective2_1 + objective2_2*self.s_T + objective2 = objective2_1 + objective2_2*self.s_T + objective2_3 # Conditional objective based on time condition = self.time < self.switch.sym diff --git a/tests/test_objective.py b/tests/test_objective.py new file mode 100644 index 00000000..5946563e --- /dev/null +++ b/tests/test_objective.py @@ -0,0 +1,158 @@ +"""Fast, solver-free tests for the objective evaluation/reconstruction logic in +agentlib_mpc.data_structures.objective. + +Unlike tests/test_examples.py, these tests do not spin up agents or run an +IPOPT solve. They build the example models directly and feed them small, +hand-built result DataFrames, so the CasADi-string reconstruction used for +objective logging/reporting can be exercised in milliseconds. The example +models are reused as-is (not duplicated here) since they already demonstrate +both the current and the deprecated ("legacy") objective notations. +""" + +import sys +import unittest +import warnings +from pathlib import Path + +import numpy as np +import pandas as pd + +# "examples" is not an installed package, only importable when the repo root +# is on sys.path. pytest adds it automatically; running this file directly +# (python tests/test_objective.py, or an editor's "Run" button) does not, so +# add it here. +sys.path.insert(0, str(Path(__file__).resolve().parents[1])) + +from agentlib_mpc.data_structures.objective import ( + _replace_subexpressions, + _replace_sq_calls, +) +from examples.one_room_mpc.physical.simple_mpc_objective_testing import ( + MyCasadiModel as ConditionalObjectiveModel, +) +from examples.one_room_mpc.physical.simple_mpc_with_time_variant_inputs import ( + MyCasadiModel as LegacyNotationModel, +) + + +def _make_result_df(index, columns): + """Build a results-style DataFrame with (col_type, name) MultiIndex columns, + matching what CombinedObjective/ConditionalObjective.calculate_values expects.""" + col_index = pd.MultiIndex.from_tuples(columns.keys()) + df = pd.DataFrame(index=index, columns=col_index, dtype=float) + for key, values in columns.items(): + df[key] = values + return df + + +class TestReplaceSubexpressions(unittest.TestCase): + """Unit tests for the @N=... subexpression inlining helper.""" + + def test_passthrough_without_at_symbol(self): + self.assertEqual(_replace_subexpressions("mDot+T_slack"), "mDot+T_slack") + + def test_definition_with_comma_containing_function_is_not_truncated(self): + # Regression test: fmin(x, y) takes two comma-separated arguments, so a + # naive "stop at the first comma" parser used to truncate the @1 + # definition to "fmin(x" and mangle the rest of the expression. + expr_str = "@1=fmin(x,y), (atan2(@1,z)+(2.*@1))" + self.assertEqual( + _replace_subexpressions(expr_str), + "(atan2((fmin(x,y)),z)+(2.*(fmin(x,y))))", + ) + + def test_nested_subexpression_references_are_resolved(self): + # @2 references @1; both must be fully inlined into the final expression. + expr_str = "@1=where(a,b,c), @2=@1+d, @2*2" + self.assertEqual( + _replace_subexpressions(expr_str), + "((where(a,b,c))+d)*2", + ) + + +class TestReplaceSqCalls(unittest.TestCase): + """Unit tests for the sq(x) -> (x)**2 rewrite helper.""" + + def test_sole_sq_call(self): + self.assertEqual(_replace_sq_calls("sq(x)"), "(x)**2") + + def test_sq_call_within_larger_expression_only_squares_its_own_argument(self): + # Regression test: squaring used to be applied to the *entire* result + # whenever "sq(" appeared anywhere in the string, so r*x + s*sq(y) + # ended up computing (r*x + s*y)**2 instead of r*x + s*y**2. + expr_str = "(r_mDot*mDot)+(s_T*sq(T_slack))" + self.assertEqual( + _replace_sq_calls(expr_str), + "(r_mDot*mDot)+(s_T*(T_slack)**2)", + ) + + def test_multiple_sq_calls_are_each_squared_independently(self): + expr_str = "sq(x)+sq(y)" + self.assertEqual(_replace_sq_calls(expr_str), "(x)**2+(y)**2") + + +class TestObjectiveEvaluation(unittest.TestCase): + """Model-level tests built directly from the example models (no agents, + no solver), so both objective notations get exercised end-to-end.""" + + def test_new_notation_conditional_objective_with_comma_regression(self): + """Covers simple_mpc_objective_testing.py: nested ConditionalObjectives, + CompositeWeights, and the comma_in_subexpression_regression term added + for the fmin() comma-parsing bug, all evaluated in one pass.""" + model = ConditionalObjectiveModel() + time = np.array([0, 100, 300, 500, 650, 900, 1200]) + df = _make_result_df( + time, + { + ("variable", "mDot"): [0.02, 0.0, 0.03, 0.005, 0.02, 0.02, 0.02], + ("variable", "T"): [298, 292, 295, 290, 294, 296, 293], + ("variable", "T_slack"): [0.0, 0.1, 0.0, 0.2, 0.0, 0.0, 0.1], + ("parameter", "r_mDot"): 1.0, + ("parameter", "r_mDot2"): 5.0, + ("parameter", "s_T"): 3.0, + ("parameter", "switch"): 600.0, + }, + ) + + with warnings.catch_warnings(): + warnings.simplefilter("error", RuntimeWarning) + values = model.objective.calculate_values(df, None) + + self.assertIn("comma_in_subexpression_regression", values) + self.assertTrue(np.isfinite(values["total"])) + + def test_old_notation_with_multiple_sq_terms(self): + """Covers simple_mpc_with_time_variant_inputs.py: the deprecated + `sum([...])` objective style, wrapped into a single legacy SubObjective + containing two separate sq(...) terms (s_T*T_slack**2 and + q_T*(T-T_set)**2). Asserts the reported value matches a hand-computed + expectation, which the sq()-squares-the-whole-sum bug would break.""" + model = LegacyNotationModel() + self.assertTrue(getattr(model.objective, "_is_legacy_wrapped", False)) + + time = np.array([0, 100, 300]) + df = _make_result_df( + time, + { + ("variable", "mDot"): [0.02, 0.01, 0.03], + ("variable", "T"): [298, 295, 293], + ("variable", "T_slack"): [0.0, 0.5, 0.2], + ("variable", "T_set"): [295, 295, 295], + ("parameter", "r_mDot"): 1.0, + ("parameter", "s_T"): 3.0, + ("parameter", "q_T"): 2.0, + }, + ) + + with warnings.catch_warnings(): + warnings.simplefilter("error", RuntimeWarning) + values = model.objective.calculate_values(df, None) + + # row0: 1*0.02 + 3*0**2 + 2*(298-295)**2 = 18.02, ts=100 + # row1: 1*0.01 + 3*0.5**2 + 2*(295-295)**2 = 0.76, ts=200 + expected_total = 18.02 * 100 + 0.76 * 200 + self.assertAlmostEqual(values["total"], expected_total) + + +if __name__ == "__main__": + unittest.main() From 883f4d34e8fb14a4055eb60ffaf9efa43650b781 Mon Sep 17 00:00:00 2001 From: Frederik Muthers Date: Wed, 16 Sep 2026 15:14:48 +0200 Subject: [PATCH 08/10] added auto wrapping for pure mx objects --- agentlib_mpc/data_structures/objective.py | 36 ++++++++++++++++++++--- 1 file changed, 32 insertions(+), 4 deletions(-) diff --git a/agentlib_mpc/data_structures/objective.py b/agentlib_mpc/data_structures/objective.py index c2f3179d..e816b482 100644 --- a/agentlib_mpc/data_structures/objective.py +++ b/agentlib_mpc/data_structures/objective.py @@ -217,6 +217,16 @@ def _replace_sq_calls(expr_str): return expr_str +def _coerce_bare_expression(obj, required_attr, wrap): + """If ``obj`` already looks like a wrapper object (i.e. it has + ``required_attr``), return it unchanged. Otherwise treat it as a bare + ``casadi.MX`` expression and wrap it using ``wrap``. + """ + if not hasattr(obj, required_attr): + return wrap(obj) + return obj + + class SubObjective: _warned_names = set() @@ -482,7 +492,12 @@ def __init__(self, *objectives, normalization: float = 1.0): *objectives: Variable number of objective terms normalization: Global normalization factor """ - self.objectives = list(objectives) + self.objectives = [ + _coerce_bare_expression( + obj, "get_weighted_expression", lambda expr: SubObjective(expressions=expr) + ) + for obj in objectives + ] self.normalization = normalization self._values = {} @@ -643,11 +658,24 @@ def __init__(self, *condition_objective_pairs, default_objective=None): and objective is a CombinedObjective default_objective: The objective to use when all conditions are False """ - self.condition_objective_pairs = condition_objective_pairs - self.default_objective = default_objective or CombinedObjective() + def _coerce_to_combined(obj): + return _coerce_bare_expression( + obj, + "get_casadi_expression", + lambda expr: CombinedObjective(SubObjective(expressions=expr)), + ) + + self.condition_objective_pairs = tuple( + (condition, _coerce_to_combined(objective)) + for condition, objective in condition_objective_pairs + ) + if default_objective is None: + self.default_objective = CombinedObjective() + else: + self.default_objective = _coerce_to_combined(default_objective) self.all_objectives = [self.default_objective] - for _, objective in condition_objective_pairs: + for _, objective in self.condition_objective_pairs: if objective not in self.all_objectives: self.all_objectives.append(objective) From 48ed20d640821916ad1f0ad091bc7a46c1dac43a Mon Sep 17 00:00:00 2001 From: Frederik Muthers Date: Fri, 18 Sep 2026 12:24:54 +0200 Subject: [PATCH 09/10] added cache for compiled expression + optimized eval call --- agentlib_mpc/data_structures/objective.py | 241 +++++++++++----------- 1 file changed, 125 insertions(+), 116 deletions(-) diff --git a/agentlib_mpc/data_structures/objective.py b/agentlib_mpc/data_structures/objective.py index e816b482..ad8bb794 100644 --- a/agentlib_mpc/data_structures/objective.py +++ b/agentlib_mpc/data_structures/objective.py @@ -217,6 +217,106 @@ def _replace_sq_calls(expr_str): return expr_str +_CASADI_REPLACEMENTS = { + "sq(": "(", + "fabs(": "abs(", + "sqrt(": "sqrt(", + "sin(": "sin(", + "cos(": "cos(", + "tan(": "tan(", + "asin(": "arcsin(", + "acos(": "arccos(", + "atan(": "arctan(", + "atan2(": "arctan2(", + "sinh(": "sinh(", + "cosh(": "cosh(", + "tanh(": "tanh(", + "asinh(": "arcsinh(", + "acosh(": "arccosh(", + "atanh(": "arctanh(", + "exp(": "exp(", + "log(": "log(", + "log10(": "log10(", + "pow(": "power(", + "floor(": "floor(", + "ceil(": "ceil(", + "sign(": "sign(", + "fmin(": "minimum(", + "fmax(": "maximum(", +} + +_NUMPY_FUNCS = { + "abs": np.abs, + "sqrt": np.sqrt, + "sin": np.sin, + "cos": np.cos, + "tan": np.tan, + "arcsin": np.arcsin, + "arccos": np.arccos, + "arctan": np.arctan, + "arctan2": np.arctan2, + "sinh": np.sinh, + "cosh": np.cosh, + "tanh": np.tanh, + "arcsinh": np.arcsinh, + "arccosh": np.arccosh, + "arctanh": np.arctanh, + "exp": np.exp, + "log": np.log, + "log10": np.log10, + "power": np.power, + "floor": np.floor, + "ceil": np.ceil, + "round": np.round, + "sign": np.sign, + "minimum": np.minimum, + "maximum": np.maximum, + "max": np.maximum, + "min": np.minimum, + "where": np.where, +} + +_CASADI_FUNCTION_NAMES = set( + [k.rstrip("(") for k in _CASADI_REPLACEMENTS] + + [v.rstrip("(") for v in _CASADI_REPLACEMENTS.values() if v.endswith("(")] + + ["round", "max", "min", "minimum", "maximum", "where", "abs", "power"] +) + + +def _compile_expression(expr): + """Translate a CasADi expression into a Python/NumPy eval string plus the + variable names it references. Depends only on the expression, never on the + data, so callers cache the result instead of redoing it per evaluation. + """ + expr_str = str(expr) + + if "@" in expr_str: + expr_str = _replace_subexpressions(expr_str) + + if "?" in expr_str: + expr_str = _replace_ternary(expr_str) + + # Replace logical NOT '!' with bitwise NOT '~' for NumPy arrays + expr_str = re.sub(r"(?=!])!(?!=)", "~", expr_str) + + eval_str = _replace_sq_calls(expr_str) + for casadi_func, replacement in _CASADI_REPLACEMENTS.items(): + if casadi_func != "sq(": # already handled by _replace_sq_calls + eval_str = eval_str.replace(casadi_func, replacement) + + # Remove outer parentheses if they wrap the entire expression + if eval_str.startswith("(") and eval_str.endswith(")"): + eval_str = eval_str[1:-1] + + var_names = [ + name + for name in re.findall(r"[a-zA-Z][a-zA-Z0-9_]*", expr_str) + if name not in _CASADI_FUNCTION_NAMES + ] + + return eval_str, var_names + + def _coerce_bare_expression(obj, required_attr, wrap): """If ``obj`` already looks like a wrapper object (i.e. it has ``required_attr``), return it unchanged. Otherwise treat it as a bare @@ -248,6 +348,7 @@ def __init__( self.expression = expressions self.weight = weight self.name = name or f"obj_{id(self)}" + self._compiled = None def __add__(self, other): """Add two objectives together to create a CombinedObjective""" @@ -306,61 +407,9 @@ def _evaluate_expression(self, expr, df): if (col_type, var_name) in df.columns: return df.loc[:, (col_type, var_name)].values[:-1] - expr_str = str(expr) - - if "@" in expr_str: - expr_str = _replace_subexpressions(expr_str) - - if "?" in expr_str: - expr_str = _replace_ternary(expr_str) - - # Replace logical NOT '!' with bitwise NOT '~' for NumPy arrays - expr_str = re.sub(r"(?=!])!(?!=)", "~", expr_str) - - # Handle common CasADi functions with simple replacements - casadi_replacements = { - "sq(": "(", - "fabs(": "abs(", - "sqrt(": "sqrt(", - "sin(": "sin(", - "cos(": "cos(", - "tan(": "tan(", - "asin(": "arcsin(", - "acos(": "arccos(", - "atan(": "arctan(", - "atan2(": "arctan2(", - "sinh(": "sinh(", - "cosh(": "cosh(", - "tanh(": "tanh(", - "asinh(": "arcsinh(", - "acosh(": "arccosh(", - "atanh(": "arctanh(", - "exp(": "exp(", - "log(": "log(", - "log10(": "log10(", - "pow(": "power(", - "floor(": "floor(", - "ceil(": "ceil(", - "sign(": "sign(", - "fmin(": "minimum(", - "fmax(": "maximum(", - } - - # Apply replacements - eval_str = _replace_sq_calls(expr_str) - - for casadi_func, replacement in casadi_replacements.items(): - if casadi_func != "sq(": # already handled above - eval_str = eval_str.replace(casadi_func, replacement) - - # Extract variable names, filtering out CasADi function names - base_funcs = [k.rstrip("(") for k in casadi_replacements.keys()] - replaced_funcs = [v.rstrip("(") for v in casadi_replacements.values() if v.endswith("(")] - extra_funcs = ["round", "max", "min", "minimum", "maximum", "where", "abs", "power"] - casadi_functions = list(set(base_funcs + replaced_funcs + extra_funcs)) - - var_names = re.findall(r"[a-zA-Z][a-zA-Z0-9_]*", expr_str) - var_names = [name for name in var_names if name not in casadi_functions] + if self._compiled is None: + self._compiled = _compile_expression(expr) + eval_str, var_names = self._compiled values_found = {} for var_name in var_names: @@ -370,49 +419,8 @@ def _evaluate_expression(self, expr, df): break try: - safe_dict = values_found.copy() - - # Handle common mathematical operations and numpy functions - safe_dict.update( - { - "abs": np.abs, - "sqrt": np.sqrt, - "sin": np.sin, - "cos": np.cos, - "tan": np.tan, - "arcsin": np.arcsin, - "arccos": np.arccos, - "arctan": np.arctan, - "arctan2": np.arctan2, - "sinh": np.sinh, - "cosh": np.cosh, - "tanh": np.tanh, - "arcsinh": np.arcsinh, - "arccosh": np.arccosh, - "arctanh": np.arctanh, - "exp": np.exp, - "log": np.log, - "log10": np.log10, - "power": np.power, - "floor": np.floor, - "ceil": np.ceil, - "round": np.round, - "sign": np.sign, - "minimum": np.minimum, - "maximum": np.maximum, - "max": np.maximum, - "min": np.minimum, - "where": np.where, - } - ) - - # Remove outer parentheses if they wrap the entire expression - if eval_str.startswith("(") and eval_str.endswith(")"): - eval_str = eval_str[1:-1] - - result = eval(eval_str, {"__builtins__": {}}, safe_dict) - - return result + safe_dict = {**values_found, **_NUMPY_FUNCS} + return eval(eval_str, {"__builtins__": {}}, safe_dict) except SyntaxError as e: if self.name not in SubObjective._warned_names: @@ -778,10 +786,12 @@ def _evaluate_condition(self, condition, df): if "?" in condition_str: condition_str = _replace_ternary(condition_str) - # Replace CasADi logical operators with Python equivalents for eval - condition_str = condition_str.replace("&&", " and ") - condition_str = condition_str.replace("||", " or ") - condition_str = re.sub(r"(?=!])!(?!=)", " not ", condition_str) + # Replace CasADi logical operators with NumPy's elementwise equivalents + # so the condition can be evaluated on whole arrays at once instead of + # per-row (Python's and/or/not only work on single truth values). + condition_str = condition_str.replace("&&", " & ") + condition_str = condition_str.replace("||", " | ") + condition_str = re.sub(r"(?=!])!(?!=)", " ~", condition_str) identifier_pattern = r'[a-zA-Z_][a-zA-Z0-9_]*' potential_vars = re.findall(identifier_pattern, condition_str) @@ -817,19 +827,18 @@ def _evaluate_condition(self, condition, df): break n_rows = len(df) - mask = np.zeros(n_rows, dtype=bool) - - for i in range(n_rows): - local_vars = {} - for var_name, values in values_dict.items(): - local_vars[var_name] = values[i] - try: - eval_str = condition_str - result = eval(eval_str, {"__builtins__": {}, "abs": abs, "min": min, "max": max, "where": np.where}, local_vars) - mask[i] = bool(result) - - except Exception as e: - print(f"Error evaluating condition at row {i}: {e}") - mask[i] = False + + try: + result = eval( + condition_str, + {"__builtins__": {}, "abs": np.abs, "min": np.minimum, "max": np.maximum, "where": np.where}, + values_dict, + ) + # A condition with no per-row variables (e.g. a constant) evaluates + # to a plain bool instead of an array; broadcast it to full length. + mask = np.broadcast_to(np.asarray(result, dtype=bool), (n_rows,)).copy() + except Exception as e: + print(f"Error evaluating condition '{condition_str}': {e}") + mask = np.zeros(n_rows, dtype=bool) return pd.Series(mask, index=df.index) From d8ca474d667b7db2231f38422f9b2a8aaa96fa13 Mon Sep 17 00:00:00 2001 From: Frederik Muthers Date: Wed, 30 Sep 2026 14:14:18 +0200 Subject: [PATCH 10/10] fixed logical operators evaluation and added tests --- agentlib_mpc/data_structures/objective.py | 58 +++++-- tests/test_objective.py | 177 ++++++++++++++++++++++ 2 files changed, 226 insertions(+), 9 deletions(-) diff --git a/agentlib_mpc/data_structures/objective.py b/agentlib_mpc/data_structures/objective.py index ad8bb794..13c2e4ce 100644 --- a/agentlib_mpc/data_structures/objective.py +++ b/agentlib_mpc/data_structures/objective.py @@ -217,6 +217,39 @@ def _replace_sq_calls(expr_str): return expr_str +def _replace_logical_ops(expr_str): + """ + Replace CasADi's (a&&b), (a||b) and (!a) with NumPy's logical_and/or/not + calls. Unlike &, | and ~, these treat any non-zero number as True, so + float/int operands (e.g. a binary input stored as 0.0/1.0) work too. + Relies on CasADi wrapping every logical operation in its own parentheses. + """ + expr_str = expr_str.replace("(!", "logical_not(") + for op, func in (("&&", "logical_and"), ("||", "logical_or")): + expr_str = f"({expr_str})" # guarantees an enclosing parenthesis + while True: + idx = expr_str.find(op) + if idx == -1: + break + # Walk back to the opening parenthesis of the enclosing (a op b) + depth = 0 + for start in range(idx - 1, -1, -1): + if expr_str[start] == ")": + depth += 1 + elif expr_str[start] == "(": + if depth == 0: + break + depth -= 1 + expr_str = ( + expr_str[:start] + + f"{func}(" + + expr_str[start + 1 : idx] + + ", " + + expr_str[idx + len(op) :] + ) + return expr_str + + _CASADI_REPLACEMENTS = { "sq(": "(", "fabs(": "abs(", @@ -274,12 +307,16 @@ def _replace_sq_calls(expr_str): "max": np.maximum, "min": np.minimum, "where": np.where, + "logical_and": np.logical_and, + "logical_or": np.logical_or, + "logical_not": np.logical_not, } _CASADI_FUNCTION_NAMES = set( [k.rstrip("(") for k in _CASADI_REPLACEMENTS] + [v.rstrip("(") for v in _CASADI_REPLACEMENTS.values() if v.endswith("(")] + ["round", "max", "min", "minimum", "maximum", "where", "abs", "power"] + + ["logical_and", "logical_or", "logical_not"] ) @@ -296,8 +333,7 @@ def _compile_expression(expr): if "?" in expr_str: expr_str = _replace_ternary(expr_str) - # Replace logical NOT '!' with bitwise NOT '~' for NumPy arrays - expr_str = re.sub(r"(?=!])!(?!=)", "~", expr_str) + expr_str = _replace_logical_ops(expr_str) eval_str = _replace_sq_calls(expr_str) for casadi_func, replacement in _CASADI_REPLACEMENTS.items(): @@ -786,12 +822,7 @@ def _evaluate_condition(self, condition, df): if "?" in condition_str: condition_str = _replace_ternary(condition_str) - # Replace CasADi logical operators with NumPy's elementwise equivalents - # so the condition can be evaluated on whole arrays at once instead of - # per-row (Python's and/or/not only work on single truth values). - condition_str = condition_str.replace("&&", " & ") - condition_str = condition_str.replace("||", " | ") - condition_str = re.sub(r"(?=!])!(?!=)", " ~", condition_str) + condition_str = _replace_logical_ops(condition_str) identifier_pattern = r'[a-zA-Z_][a-zA-Z0-9_]*' potential_vars = re.findall(identifier_pattern, condition_str) @@ -831,7 +862,16 @@ def _evaluate_condition(self, condition, df): try: result = eval( condition_str, - {"__builtins__": {}, "abs": np.abs, "min": np.minimum, "max": np.maximum, "where": np.where}, + { + "__builtins__": {}, + "abs": np.abs, + "min": np.minimum, + "max": np.maximum, + "where": np.where, + "logical_and": np.logical_and, + "logical_or": np.logical_or, + "logical_not": np.logical_not, + }, values_dict, ) # A condition with no per-row variables (e.g. a constant) evaluates diff --git a/tests/test_objective.py b/tests/test_objective.py index 5946563e..319e0a56 100644 --- a/tests/test_objective.py +++ b/tests/test_objective.py @@ -14,6 +14,7 @@ import warnings from pathlib import Path +import casadi as ca import numpy as np import pandas as pd @@ -24,6 +25,10 @@ sys.path.insert(0, str(Path(__file__).resolve().parents[1])) from agentlib_mpc.data_structures.objective import ( + CombinedObjective, + ConditionalObjective, + SubObjective, + _replace_logical_ops, _replace_subexpressions, _replace_sq_calls, ) @@ -154,5 +159,177 @@ def test_old_notation_with_multiple_sq_terms(self): self.assertAlmostEqual(values["total"], expected_total) +class TestBareMxAutoWrapping(unittest.TestCase): + """Regression tests for mixing raw casadi.MX expressions with wrapper + objects (SubObjective/CombinedObjective) inside CombinedObjective and + ConditionalObjective. + + CasadiModel.__init__ already auto-wraps a single bare MX at the root of + the objective tree (backwards-compat shim for the deprecated notation). + This used to be the *only* place that happened, so mixing a bare MX + branch (e.g. from plain `sum([...]) / normalization` arithmetic) with a + wrapper branch built via create_combined_objective(...)/ + create_conditional_objective(...) raised AttributeError deep inside + get_casadi_expression(). These tests cover the same auto-wrap now + happening at every level of the tree. + """ + + def test_combined_objective_mixes_bare_mx_and_sub_objective(self): + x = ca.MX.sym("x") + y = ca.MX.sym("y") + wrapped = SubObjective(expressions=y, weight=2, name="wrapped") + + combined = CombinedObjective(x, wrapped, normalization=1.0) + + self.assertIsInstance(combined.objectives[0], SubObjective) + self.assertIs(combined.objectives[1], wrapped) + self.assertIsInstance(combined.get_casadi_expression(), ca.MX) + + def test_conditional_objective_mixes_bare_mx_and_combined_objective(self): + x = ca.MX.sym("x") + y = ca.MX.sym("y") + condition = ca.MX.sym("cond") + + combined_branch = CombinedObjective(SubObjective(expressions=y)) + + conditional = ConditionalObjective( + (condition, x), default_objective=combined_branch + ) + + self.assertIsInstance(conditional.get_casadi_expression(), ca.MX) + + def test_pure_wrapper_usage_is_unaffected(self): + x = ca.MX.sym("x") + sub = SubObjective(expressions=x, weight=1, name="sub") + + combined = CombinedObjective(sub, normalization=1.0) + + # Already a wrapper object -- must be stored unchanged, not re-wrapped. + self.assertIs(combined.objectives[0], sub) + self.assertIsInstance(combined.get_casadi_expression(), ca.MX) + + def test_pure_bare_mx_default_objective_still_works(self): + # Mirrors the pre-existing root-level wrap in CasadiModel.__init__: a + # bare MX given as the sole (default) objective should still work. + x = ca.MX.sym("x") + + conditional = ConditionalObjective(default_objective=x) + + self.assertIsInstance(conditional.default_objective, CombinedObjective) + self.assertIsInstance(conditional.get_casadi_expression(), ca.MX) + + +class TestReplaceLogicalOps(unittest.TestCase): + """Unit tests for the &&/||/! -> logical_and/or/not rewrite helper.""" + + def test_and(self): + self.assertEqual( + _replace_logical_ops("((1 1, self.y < 2) + self.assertEqual(self._mask(condition), [False, False, True, False]) + + def test_and_with_float_operand(self): + condition = ca.logic_and(self.u, self.x > 1) + self.assertEqual(self._mask(condition), [False, True, False, False]) + + def test_not_of_float_operand(self): + condition = ca.logic_not(self.u) + self.assertEqual(self._mask(condition), [False, False, True, True]) + + def test_nested_or_and_with_float_operand(self): + condition = ca.logic_or(self.x > 1, ca.logic_and(self.y < 2, self.u)) + self.assertEqual(self._mask(condition), [True, True, True, False]) + + def test_nonzero_operand_other_than_one_is_true(self): + # 2 is logically True, even though 2 & True == 0 bitwise. + condition = ca.logic_and(self.k, self.u > 0.5) + self.assertEqual(self._mask(condition), [True, False, False, False]) + + def test_not_inside_if_else_subexpression(self): + # CasADi prints this with an @1 subexpression, a ternary and a (!@1). + condition = ca.if_else(self.x > 1, self.y, self.u) > 0.5 + self.assertEqual(self._mask(condition), [True, True, True, False]) + + +class TestSubObjectiveLogicalOps(unittest.TestCase): + """Regression tests for &&/||/! inside SubObjective expressions. &&/|| + used to be left untranslated, so the term hit a SyntaxError and was + logged as 0, and ! became ~, which raised a TypeError on float data.""" + + def setUp(self): + self.x, self.y, self.u = (ca.MX.sym(n) for n in "xyu") + # Last row is dropped by the evaluation; ts is 100 for every step. + self.df = _make_result_df( + np.array([0, 100, 200, 300]), + { + ("variable", "x"): [0.5, 1.5, 1.5, 0.5], + ("variable", "y"): [1.0, 3.0, 1.0, 1.0], + ("variable", "u"): [1.0, 1.0, 0.0, 0.0], + }, + ) + + def _value(self, expression, name): + objective = SubObjective(expressions=expression, name=name) + with warnings.catch_warnings(): + warnings.simplefilter("error", RuntimeWarning) + return objective.calculate_value(self.df, 1) + + def test_and_of_comparisons(self): + expression = ca.if_else(ca.logic_and(self.x > 1, self.y < 2), self.x, 0) + self.assertAlmostEqual(self._value(expression, "and_cmp"), 1.5 * 100) + + def test_and_with_float_operand(self): + expression = ca.if_else(ca.logic_and(self.u, self.x > 1), self.y, 0) + self.assertAlmostEqual(self._value(expression, "and_float"), 3.0 * 100) + + def test_or_with_float_operand(self): + expression = ca.if_else(ca.logic_or(self.u, self.x > 1), self.y, 0) + self.assertAlmostEqual(self._value(expression, "or_float"), 5.0 * 100) + + def test_not_of_float_operand(self): + expression = ca.if_else(ca.logic_not(self.u), self.y, 0) + self.assertAlmostEqual(self._value(expression, "not_float"), 1.0 * 100) + + if __name__ == "__main__": unittest.main()