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

Enabeling opty to estimate input time shifts. #294

Draft
wants to merge 26 commits into
base: master
Choose a base branch
from

Conversation

chris-konrad
Copy link
Contributor

@chris-konrad chris-konrad commented Jan 3, 2025

This is work in progress addressing #224. The code currently does not run without exceptions and added functions are untested.

Motivation

For a dynamic system $\dot{x}(t) = f(x(t)) + u(t-\tau)$, one may want to estimate the unknown input delay $\tau$. Currently, opty does not support timeshift symbols and hence can't explicitly estimate this delay (See #224).

Method

A potential way to enable opty to find the desired $\tau$ directly is outlined as follows (credits to @moorepants)

  • Substitute $u(t-\tau) = u'(t)$ and consider $u'(t)$ an unknown trajectory and $\tau$ an unknown free parameter.
  • Consider $u(t)$ a known trajectory.
  • Create instance constraints $c_i(u'[i], \tau) = u'[i] - u[i-\frac{\tau}{\Delta t}]$ for all collocation nodes enforcing that.

Calculating the constrain Jacobian

The timeshift constraints have to be added to the constraint jacobian. For the constraints $c_i$ as defined above, the Jacobian is

  • for midpoint integration:
    $G_i = [\frac{\partial c_i}{\partial u[i]}, \frac{\partial c_i}{\partial \tau}] = [1, \frac{u'[i-\frac{\tau}{\Delta t}+1] - u'[i-\frac{\tau}{\Delta t}-1]}{2\Delta t} \cdot \frac{1}{\Delta t}]$

  • for backwards euler integration:
    $G_i = [\frac{\partial c_i}{\partial u[i]}, \frac{\partial c_i}{\partial \tau}] = [1, \frac{u'[i-\frac{\tau}{\Delta t}] - u'[i-\frac{\tau}{\Delta t}-1]}{\Delta t} \cdot \frac{1}{\Delta t}]$

Status

Development is ongoing in branch input-time-shifts. The current code does not run without exceptions and the added functions are untested.

TODO List

  • Add a function to identify and substitute trajectories with a timeshift (16cc611, 89b7db3)
  • Verify that timeshift trajectories are given in the correct format (6f79482)
  • Require unshifted input to be a known trajectory (11ab07d)
  • Generate instance constraints (89b7db3)
  • Enable _find_closest_free_index() to create the indices of constraints with timeshift (b52a96f, 1e9a710, 11ab07d)
  • Enable _instance_constraints_func() to handle timeshift constraints. (b52a96f, 1e9a710, 11ab07d)
  • Enable _instance_constraints_jacobian_indices() to handle timeshift constraints (6f79482).
  • Enable jacobian_indices() to find the dynamic indices of timeshift constraints
  • Test if constraints_jacobian() needs to be adressed
  • Make sure that existing unittests succeed
  • Add bounds to timeshift parameters and process known trajectories with different lengths
  • Write unittests for new functionality.
  • Test new functionalitiy

@moorepants
Copy link
Member

See chris-konrad#1 for status and plans.

This would be nice if were an issue here, because forks are often temporary or just may disappear at some point.

@chris-konrad
Copy link
Contributor Author

This would be nice if were an issue here, because forks are often temporary or just may disappear at some point.

I added it to the first message in this PR

@chris-konrad
Copy link
Contributor Author

chris-konrad commented Jan 4, 2025

I added the new indices of the new timeshift contraint partials to the jacobian indices. This assumes the following layout of the Jacobian. Can you confirm that this is correct, @moorepants? Green is the existing Jacobian, yellow is the extension with timeshift constraints.

The constraint definition is as above:
$G_i = [\frac{\partial c_i}{\partial u[i]}, \frac{\partial c_i}{\partial \tau}]$

constraint_jacobian_structure drawio

Creating a new problem with Problem() now runs through without exceptions. I did not test it any further, so far.

@Peter230655
Copy link
Contributor

Peter230655 commented Jan 4, 2025

@chris-konrad : Curiosity: Once this works will opty be able to handle time delayed eoms, like $\dfrac{dx(t)}{dt} = f(x(t - \tau), u(x - \tau), parameters)$ ?
Thanks!

@chris-konrad
Copy link
Contributor Author

chris-konrad commented Jan 5, 2025

@Peter230655: No, unfortunately this will not directly be possible. The implemented approach requires that the timeshifted trajectory is a known trajectory.

@Peter230655
Copy link
Contributor

@Peter230655: No, unfortunately this will not directly be possible. The implemented approach requires that the timeshifted trajectory is a known trajectory.

Thanks!
Still good for me: I am mostly a user of sympy mechanics and of opty, playing around with them to pass time in retirement.
This, when implemented will enlarge the playing field for me. :-)

@chris-konrad
Copy link
Contributor Author

chris-konrad commented Jan 5, 2025

All previously existing unittests now run fine but it seems to break some examples with variable times. This is little surprising as I did not pay any attention to that during implementation. Before addressing these issues, I will focus on testing if the new functions work at all.

@moorepants
Copy link
Member

This assumes the following layout of the Jacobian. Can you confirm that this is correct, @moorepants?

Are you treating $\tau$ the same as any unknown parameter? i.e. is it in the list of unknown_parameters? This would have some bearing on the construction of the jacobian (order of rows).

self.eom = sym.Matrix(((x_1(t).diff() - x_2(t),),
(x_2(t).diff() + a_0 * T_0 * x_1(t)
+ a_1 * T_0 * x_2(t)
- T_0 * u(t-t_d),)))
Copy link
Member

Choose a reason for hiding this comment

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

Why would u_shift not be added here instead of u(t-t_d)? Shouldn't the eom's not have t_d present?

Copy link
Contributor Author

@chris-konrad chris-konrad Jan 6, 2025

Choose a reason for hiding this comment

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

This is the eom that I pass to the constructor of Problem. It has t_d present in u(t-t_d). The ConstraintCollocator then looks through this eom, finds all expressions of the form U(time_symbol +/- symbol) and substitutes them with U_shift(time_symbol (see: ConstraintCollocator._substitute_timeshift_trajectories(self))

self.state_symbols = (x_1, x_2)
self.constant_symbols = (a_0, a_1)
self.constant_values = (1, np.sqrt(2))
self.timeshift_inputs = {u_shift(t): (u(t - t_d), t_d)}
Copy link
Member

Choose a reason for hiding this comment

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

Why did you choose adding a new attribute here, instead of passing u_shift - u(t - t_d) directly to instance constraints?

Copy link
Contributor Author

@chris-konrad chris-konrad Jan 6, 2025

Choose a reason for hiding this comment

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

After substituting the timeshift trajectories, ConstraintCollocator automatically appends instance constraints representing u_shift(t) - u(t - t_d) to the other instance constraints (see: ConstraintCollocator._generate_timeshift_constraints()) , I just didn't write a test for this yet. The attribute ConstraintCollocator.timeshift_trajectory_substitutes, that I check in this test, is just a map from the substitutes to the original trajectory and the timeshift parameter that I occasionally need throughout the code to look up which functions and parameters relate to the timeshift.

@chris-konrad
Copy link
Contributor Author

This assumes the following layout of the Jacobian. Can you confirm that this is correct, @moorepants?

Are you treating τ the same as any unknown parameter? i.e. is it in the list of unknown_parameters? This would have some bearing on the construction of the jacobian (order of rows).

Thats what I do. Indeed there seems to be a problem in the Jacobian index function related to this. I will have a look.

@chris-konrad chris-konrad marked this pull request as draft January 7, 2025 13:32
…pecified values for the case of single unknown trajectories
@@ -1680,6 +1958,9 @@ def constraints(free):
eom_con_vals = func(free_states, all_specified, all_constants,
time_interval)

if all_specified.shape[0] == 1:
all_specified.squeeze()
Copy link
Member

Choose a reason for hiding this comment

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

Should be safe to call squeeze() without the if statement, as it will only affect arrays with an empty dimension.

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.

3 participants