-
Notifications
You must be signed in to change notification settings - Fork 21
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
time_vector method added. Should Fix #316 #318
base: master
Are you sure you want to change the base?
Conversation
This needs to resolve conflicts. |
opty/direct_collocation.py
Outdated
@@ -686,6 +686,35 @@ def parse_free(self, free): | |||
|
|||
return parse_free(free, n, q, N, variable_duration) | |||
|
|||
def time_vector(self, t0=0.0, solution=None): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
def time_vector(self, t0=0.0, solution=None): | |
def time_vector(self, solution=None, start_time=0.0): |
I think people will rarely use the start time kwarg, so make it second. And rename to have more informative variable name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I now wonder, whether we should give t0 at all: I would not know how to tell opty not to start at t= = 0.0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can leave it off and add it later if we ever think we need it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can leave it off and add it later if we ever think we need it.
I left it in. Do you want me to remove it?
opty/direct_collocation.py
Outdated
A numpy num_collocation_nodes-array of time instances. | ||
|
||
""" | ||
if isinstance(self.collocator.node_time_interval, sm.Symbol): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a flag self.collocator._variable_duration
that should be used for this check.
opty/direct_collocation.py
Outdated
msg = 'Solution vector must be provided for variable duration.' | ||
raise ValueError(msg) | ||
else: | ||
return np.arange(t0, t0+self.collocator.num_collocation_nodes* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return np.arange(t0, t0+self.collocator.num_collocation_nodes* | |
return np.arange(t0, t0 + self.collocator.num_collocation_nodes* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When do you put a space and when do you not put a space?
I thought in signatures one does not put a space?
Of course I will correct it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PEP8 has the guidelines.
opty/direct_collocation.py
Outdated
solution[-1], solution[-1]) | ||
|
||
else: | ||
return np.arange(t0, t0+self.collocator.num_collocation_nodes* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return np.arange(t0, t0+self.collocator.num_collocation_nodes* | |
return np.arange(t0, t0 + self.collocator.num_collocation_nodes* |
This needs a unit test. |
Added identical time_vector method in Problem and in ConstraintCollocator. Should fix issue #316