add parse_free method to Problem class, add test fubction #303
Conversation
opty/direct_collocation.py
Outdated
| len_states = n * N | ||
| len_specified = q * N | ||
|
|
||
| free_states = free[:len_states].reshape((n, N)) | ||
|
|
||
| if q == 0: | ||
| free_specified = None | ||
| else: | ||
| free_specified = free[len_states:len_states + len_specified] | ||
| if q > 1: | ||
| free_specified = free_specified.reshape((q, N)) | ||
|
|
||
| if variable_duration: | ||
| free_time_interval = free[-1] | ||
| free_constants = free[len_states + len_specified:-1] | ||
| return free_states, free_specified, free_constants, free_time_interval | ||
| else: | ||
| free_constants = free[len_states + len_specified:] | ||
| return free_states, free_specified, free_constants |
There was a problem hiding this comment.
| len_states = n * N | |
| len_specified = q * N | |
| free_states = free[:len_states].reshape((n, N)) | |
| if q == 0: | |
| free_specified = None | |
| else: | |
| free_specified = free[len_states:len_states + len_specified] | |
| if q > 1: | |
| free_specified = free_specified.reshape((q, N)) | |
| if variable_duration: | |
| free_time_interval = free[-1] | |
| free_constants = free[len_states + len_specified:-1] | |
| return free_states, free_specified, free_constants, free_time_interval | |
| else: | |
| free_constants = free[len_states + len_specified:] | |
| return free_states, free_specified, free_constants | |
| return parse_free(n, q, N, variable_duration) |
This is indeed a nice feature. You can remove some code duplicaton by just calling the parse_free function.
There was a problem hiding this comment.
Thanks!
I thought of this but I was not sure util's parse_free is automatically available.
Does it not have to be 'imported'?
There was a problem hiding this comment.
Yes, it needs to be imported, which you can see is done already at the top of this module.
There was a problem hiding this comment.
Now I noticed.
So I am 'allowed' to use util's parse_free?
There was a problem hiding this comment.
Yes, it is even preferable as it reduces code duplication, i.e. same code in multiple locations.
There was a problem hiding this comment.
I already changed as per your suggestion.
Somehow, I feel 'more comfortable' with my clumpsy test function. I can't really say why. I know yours is correct.
There was a problem hiding this comment.
Yes, it is even preferable as it reduces code duplication, i.e. same code in multiple locations.
Thanks!
This really helped my understanding on such packages!
| @@ -1566,3 +1566,125 @@ def test_for_algebraic_eoms(): | |||
| ) | |||
|
|
|||
| assert excinfo.type is ValueError | |||
|
|
|||
|
|
|||
| def test_prob_parse_free(): | |||
There was a problem hiding this comment.
This test can be made way simpler. You can take some inspiration from test_parse_free. You should be able to relatively easily just create some random equations which give a certain n, q, r, N.
There was a problem hiding this comment.
To create a simple equation of n symbols, one can use something like sum(sm.symbols(f"p:{n}")).
There was a problem hiding this comment.
If you'd like it to have a physical meaning. You can just imagine a set of n independent point masses that can only move in one direction by example.
There was a problem hiding this comment.
This is VERY clever indeed!
|
Simplified completely as per Timo's suggestion! |
| states, controls, constants = prob.parse_free(initial_guess) | ||
| assert(np.all(states == statesu)) | ||
| assert(np.all(controls == controlsu)) | ||
| assert(np.all(constants == constantsu)) |
There was a problem hiding this comment.
The better testing for floating point comparison is np.testing.test_allclose().
There was a problem hiding this comment.
Sorry, it is np.testing.assert_allclose().
|
I used assert(np.allclose(a, b)) |
|
Question: |
|
This looks good. Thanks! |
Thanks! :-) |
I added parse_free(solution) to the Problem class, and added a test function comparing the new method to the one existing in utils. This should take care of issue #203