Skip to content

95 objective evaluation for complex objective - #132

Open
frederikmuthers wants to merge 11 commits into
mainfrom
95-objective-evaluation-for-complex-objective
Open

frederikmuthers wants to merge 11 commits into
mainfrom
95-objective-evaluation-for-complex-objective

Conversation

@frederikmuthers

Copy link
Copy Markdown
Collaborator

Pull Request

Description

Added functionality to clean up objectives in string format to make them compatible with the python eval functionality.

Type of Change

  • added _replace_subexpression
  • added _replace_ternary
  • extended number of possible functions (probably excessive)
  • made "casadi_functions" to be derived from "casadi_expressions"
  • added exclusion of old notation from objective evaluation via legacy flag
  • added dedicated example for testing of objective functions

@frederikmuthers frederikmuthers self-assigned this Apr 30, 2026
@sarahleidolf
sarahleidolf requested a balanced review from Copilot August 13, 2026 09:38

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@sarahleidolf
sarahleidolf requested a balanced review from Copilot August 13, 2026 09:39

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Suppressed comments (4)

examples/one_room_mpc/physical/simple_mpc_objective_testing.py:298

  • if with_dashboard: is duplicated, creating an unnecessary nested conditional and indentation. This is likely accidental and makes the example harder to read.
    if with_dashboard:
        if with_dashboard:
            from agentlib_mpc.utils.analysis import load_mpc_stats

agentlib_mpc/data_structures/objective.py:28

  • _replace_subexpressions() uses a regex that stops each @n=... definition at the first comma, so any subexpression containing commas (e.g., function calls like if_else(..., ..., ...)) will be truncated and inlined incorrectly, leading to invalid/wrong eval strings.
    # 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)

examples/one_room_mpc/physical/simple_mpc_objective_testing.py:176

  • This block assigns to condition repeatedly, so only the final assignment is actually used. This makes the example misleading (it looks like multiple cases are being tested, but they are dead code).

This issue also appears on line 296 of the same file.

        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

examples/one_room_mpc/physical/simple_mpc_objective_testing.py:329

  • Spelling: "Absoulute" should be "Absolute".
    print(f"Absoulute integral error: {aie_kh} Kh.")

@sarahleidolf sarahleidolf left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Add new objective function to ci tests
  2. If possible, reduce the computation time required to calculate the objective values

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

New objective calculation crashes for more complex objective functions

3 participants