Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

args and kwargs for create_objective_function #122

Closed
moorepants opened this issue Feb 12, 2024 · 5 comments · Fixed by #201
Closed

args and kwargs for create_objective_function #122

moorepants opened this issue Feb 12, 2024 · 5 comments · Fixed by #201
Milestone

Comments

@moorepants
Copy link
Member

Current is:

def create_objective_function(
    objective, state_symbols, input_symbols, unknown_symbols,
    N, node_time_interval=1.0, integration_method="backward euler"):

What is required:

  • objective
  • state symbols
  • N
  • node_time_interval

what is optional:

  • input symbols
  • unknown symbols
  • integration method

You need either input symbols or unknown symbols and the input symbols are any functions of time that are not state symbols, so they can actually be detected. A pure parameter id problem only needs unknown symbols, not input symbols and a pure open loop trajectory optimization only needs input_symbols.

The time interval always has to be specified and the chances that it is 1.0 is very low, so I think it should be an arg.

Technically the integration method is required so an arg may be best, but given that the default is backward euler in Problem, it is probably fine to keep it this way.

My suggestion would be to adjust the args and kwargs to make:

  • input symbols automatically detected
  • unknown symbols a kwarg
  • time interval an arg
  • and maybe integration method an arg
@tjstienstra
Copy link
Contributor

The problem with automatic detection is that you only know the objective and some variables may be missing from there. If one unknown trajectory misses, then you by definition don't know what your free vector will look like. Similar for the unknown parameters.

The reason for interval_value to be a kwarg is that it only scales the problem.

@moorepants
Copy link
Member Author

The problem with automatic detection is that you only know the objective and some variables may be missing from there. If one unknown trajectory misses, then you by definition don't know what your free vector will look like. Similar for the unknown parameters.

Yep, didn't think of that. I think that the later two should be kwargs still though, otherwise you have to pass tuple() when you want it to be effectively None.

@tjstienstra
Copy link
Contributor

Another option would be to change the signature of Problem. This could also enhance user experience. This could look a bit like this:

# Assume obj and obj_grad are passed
obj = args[0] if args else kwargs.get("obj")
obj_grad args[1] if len(args)>1 else kwargs.get("obj_grad")
It "obj_expr" in kwargs:
    if isinstance(args[0], Callable) or isinstance(args[1], Callable):
         raise ValueError("The objective function should be provided either as expression ('obj_expr')or as two evaluation functions ('obj', 'obj_grad').")
    obj, obj_grad = create_objective_function(...)
else:
    args = args[2:]  # Something like this

@moorepants
Copy link
Member Author

I'm open to that.

@moorepants
Copy link
Member Author

But, I suspect the function may fail if people throw more random objective functions at it. We need to be clear on what it can and can't do.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants