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

Implements support for variable duration solutions #150

Merged
merged 15 commits into from
May 10, 2024

Conversation

moorepants
Copy link
Member

@moorepants moorepants commented May 5, 2024

Fixes #19

This PR allows a user to pass in a symbol as the node interval value instead of a float and then specify instance constraints in terms of node index instead of time. This adds the node interval value as a free optimization variable (as the last element) and the resulting constraint equations and Jacobian takes into account the variable interval. I demonstrate it on the pendulum swing up problem.

  • Allow user to set bounds on the node time interval.
  • Update parse_free to deal with having h as the last term.
  • Make example problem for variable duration.
  • Show how to use this in the documentation examples.

@moorepants moorepants marked this pull request as ready for review May 5, 2024 09:45
@moorepants
Copy link
Member Author

@tjstienstra, if you're interested in reviewing this, it is much appreciated. I've only tried it out on the example pendulum problem here, nothing more complicated yet.

@moorepants
Copy link
Member Author

If you have any comments on the API for the instance constraints, I'd be interested. The basic idea is that if you have fixed duration problems you give something like x(1.23) where the argument is time and then I find the nearest node index. If you have a variable duration, then you give x(4) as the 4th node as the argument. I'm not sure this is the best approach.

@moorepants
Copy link
Member Author

Variable duration pendulum swing up solution:
image

@tjstienstra
Copy link
Contributor

tjstienstra commented May 6, 2024

If you have any comments on the API for the instance constraints, I'd be interested.

I would have a preference not to use round brackets substituting t for indices. That may not just be confusing at first, especially since we normally find the index for the supplied time value, but it will also cause problems for possible future features.

The closest solution would be to use a multiple of h, e.g. 4*h when selecting the fifth node.

@tjstienstra
Copy link
Contributor

One of my main curiosities is also what would the API look like if variable time steps were supported?



# Specify the objective function and it's gradient.
def obj(free):
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not use the create_objective_function?

Copy link
Member Author

Choose a reason for hiding this comment

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

I wanted to write it out to see how the new variable affected the equations and did not know if create_objective_function() supported h as a symbol.

Copy link
Contributor

Choose a reason for hiding this comment

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

Not yet but we can easily make it support it (will have to check ones more). But I think that the only difference is that we should change the default keyword argument to an integer (bit nicer if someone doesn't specify it) and we have to multiply the objective with the node_time_interval and remove it from the functions where it manually multiplies the solution with the node time interval.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll let you update it after I merge this. Or I could merge in changes onto this branch if you want.

@moorepants
Copy link
Member Author

I would have a preference not to use round brackets substituting t for indices.

I suspect I could support something like theta[0] but that could take considerably use more work. I'm using (or abusing) SymPy Function behavior. I guess you could create an sympy.Indexed.

The closest solution would be to use a multiple of h, e.g. 4*h when selecting the fourth node.

I originally wrote it like this and then realized it was much simpler to ask the node number from the user, so switched to it.

@moorepants
Copy link
Member Author

One of my main curiosities is also what would the API look like if variable time steps were supported?

I guess the API wouldn't really change other than free being made longer with all the h values. We could have a flag fixed_interval_size or something.

@tjstienstra
Copy link
Contributor

tjstienstra commented May 6, 2024

I suspect I could support something like theta[0] but that could take considerably use more work.

Yeah, that was the other thing I thought of.

The closest solution would be to use a multiple of h, e.g. 4*h when selecting the fourth node.

I originally wrote it like this and then realized it was much simpler to ask the node number from the user, so switched to it.

I would prefer the additional difficulty, because it is clearer and mathematically correct. t=4h for the fifth node.

Another solution is to create a boolean keyword argument to interpret supplied time values as indices.

@tjstienstra
Copy link
Contributor

One of my main curiosities is also what would the API look like if variable time steps were supported?

I guess the API wouldn't really change other than free being made longer with all the h values. We could have a flag fixed_interval_size or something.

I was more thinking toward supplying an iterable as node_time_interval, which may include multiples of h.

@moorepants
Copy link
Member Author

I was more thinking toward supplying an iterable as node_time_interval, which may include multiples of h.

I'm not sure why you'd want to do that. What is the reason that you'd want to specify unequal proportional spacing? Wouldn't it mean you are trying to guess where the solution is stiff (smaller spacing in stiff areas) without knowing the solution?

@tjstienstra
Copy link
Contributor

I was more thinking toward supplying an iterable as node_time_interval, which may include multiples of h.

I'm not sure why you'd want to do that. What is the reason that you'd want to specify unequal proportional spacing? Wouldn't it mean you are trying to guess where the solution is stiff (smaller spacing in stiff areas) without knowing the solution?

If a problem is partially reusable then we could start iterating to improve the solution. A bit like what pycollo does only then way simpler. In several problems you know where you want a bit higher resolution and where you do not need that many points.

@moorepants
Copy link
Member Author

I see, yes, if you have a sense of the solution you could proportional time to reflect that.

@moorepants
Copy link
Member Author

I switched to integer multiples of the time interval symbol.

@moorepants moorepants merged commit 5c8fb56 into csu-hmc:master May 10, 2024
12 checks passed
@moorepants moorepants deleted the variable-duration branch May 10, 2024 08:28
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.

Implement h as a free variable
2 participants