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

Flow with variables (NonFixed ocp) #15

Closed
jbcaillau opened this issue May 18, 2024 · 9 comments · Fixed by #38
Closed

Flow with variables (NonFixed ocp) #15

jbcaillau opened this issue May 18, 2024 · 9 comments · Fixed by #38
Assignees
Labels
bug Something isn't working

Comments

@jbcaillau
Copy link
Member

jbcaillau commented May 18, 2024

@ocots the function returned by Flow applied to an ocp should depend on the variable (when there is one, i.e. when the ocp is NonFixed) For instance, in this case (where tf is variable).

@ocots
Copy link
Member

ocots commented May 20, 2024

@jbcaillau Actually, you can always give or not an extra variable to the function returned by Flow:

function f(t0::Time, x0::State, p0::Costate, tf::Time, v::Variable=__variable(); kwargs...)

If the problem is Fixed then the variable is not used (this case should be unauthorized). If the problem is NonFixed then the variable is used. In the case of the Goddard problem, there is a variable (the final time tf) that we can pass to the function returned by Flow (or not). Note that this variable is not used to compute neither the dynamics/cost nor the feedback control hence we are not compelled to pass it. In this case the variable takes the default value which is Real[]. I understand that this can be weird but it would be strange to have to write something like that:

xf, pf = f0(t3, x3, p3, tf, tf)

There is a test where you can see that we can provide a variable (I need to add tests):

xf, pf = z(t0, x0, p0, tf, -1.0)

@jbcaillau
Copy link
Member Author

jbcaillau commented May 20, 2024

Thanks @ocots. Exactly. Two points below.

Actually, you can always give or not an extra variable to the function returned by Flow:

function f(t0::Time, x0::State, p0::Costate, tf::Time, v::Variable=__variable(); kwargs...)

If the problem is Fixed then the variable is not used (this case should be unauthorized).

Point 1. Yes, when the problem is Fixed, calling a flow with a variable should not be possible.

If the problem is NonFixed then the variable is used. In the case of the Goddard problem, there is a variable (the final time tf) that we can pass to the function returned by Flow (or not). Note that this variable is not use to compute neither the dynamics/cost nor the feedback control hence we are not compelled to pass it. In this case the variable takes the default value which is Real[]. I understand that this can be weird but it would be strange to have to write something like that:

xf, pf = f0(t3, x3, p3, tf, tf)

Point 2. From the user point of view, the two particular cases when either t0 or tf is the only variable, it is strange to force the user to write such a thing. So it is good to give the user the possibility not to add the variable t0 or tf during the call in these two specific cases (dim $1$ variable equal to either t0 or tf). This can easily be taken care of with a check in __variable helper. The typical situation being the free final time case. Conversely, I would not advocate talking care of the case when the variable is a dim $2$ one and equals [ t0, tf ].

There is a test where you can see that we can provide a variable (I need to add tests):

xf, pf = z(t0, x0, p0, tf, -1.0)

  • the standard behaviour is (i) to forbid (= throw an exception) passing a variable to a Fixed problem, (ii) to force passing a variable to a NonFixed = Variable problem
  • two exceptions to (ii): for a dim $1$ variable equal either to t0 or tf, allow not to give t0 or tf when calling a flow
  • note that when t0 or tf is the (only) variable, passing Real[] as the default when the variable is omitted does not work since the computation of the flow might actually depend on the variable (tf in the dynamics, for instance...) so the value (of t0 or tf) must be retrieved by __variable

Side notes:

  • add alias Variable = NonFixed, keeping NonFixed for compatibility and homogeneity with NonAutonomous. Conversely, I do not think relevant to add the alias NonVariable = Fixed...
  • being Autonomous and / or Variable are traits and should be implemented as such (to be done)

@ocots
Copy link
Member

ocots commented Jun 22, 2024

@jbcaillau top priority.

@jbcaillau
Copy link
Member Author

@ocots agreed. I thought you would do it, but I can take care of it: let me know. In any case, I first need to smoothen some things in control-toolbox/CTBase.jl#166 (comment)

@ocots
Copy link
Member

ocots commented Jun 22, 2024

Actually I forgot. But I can take care of it.

@jbcaillau
Copy link
Member Author

jbcaillau commented Jun 22, 2024

Actually I forgot. But I can take care of it.

maybe you can. maybe not. 🤭

@ocots
Copy link
Member

ocots commented Jul 9, 2024

@jbcaillau Still forgotten.

@jbcaillau
Copy link
Member Author

jbcaillau commented Jul 9, 2024

@ocots no worry. fine as is, but we ought to take care of it these days.

@ocots ocots added the bug Something isn't working label Jul 26, 2024
@ocots ocots linked a pull request Aug 6, 2024 that will close this issue
@ocots
Copy link
Member

ocots commented Aug 6, 2024

@jbcaillau Corrected, cf. #38.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants