API Refactor: Multi revolution Lambert problem - poliastro/poliastro GitHub Wiki
Regardless of the internal implementation, the user should be able to do this in any case:
for va, vb in lambert(k, r0, r, tof):
...
Behavior for single solution
When there is only one solution, there are two options:
- Be simple, and return a single pair.
# Single solution
va, vb = lambert(k, r0, r, tof)
# Multiple solutions
(va, vb), *_ = lambert(k, r0, r, tof)
va, vb = lambert(k, r0, r, tof)[0]
- Be consistent, always.
# Single solution
(va, vb), *_ = lambert(k, r0, r, tof)
va, vb = lambert(k, r0, r, tof)[0]
# Multiple solutions
(va, vb), *_ = lambert(k, r0, r, tof)
va, vb = lambert(k, r0, r, tof)[0]
Sorting of the solutions
Which solutions should be returned first? I guess it doesn't matter as long as it is intuitive or, at least, consistent. What does the user expect as a first solution?
As it is written now, the algorithm will compute the solutions in decreasing order of revolutions and from right to left in the T-x diagram (see https://github.com/Juanlu001/poliastro/blob/lambert-izzo/examples/Revisiting%20Lambert's%20problem%20in%20Python.ipynb).
Idea: reconstruct orbital energy from x
. Is there a straightforward formula?
Number of revolutions
I see two options here:
- Specify them when calling the function. I haven't taken this approach into consideration so far, and would indeed simplify the API but not find all the possible solutions.
- Return it as part of the solution. Otherwise there's no way to know how many revolutions does a solution involve.
If N
is to be specified as part of the function call, then the block
https://github.com/Juanlu001/poliastro/blob/cf820d6/src/poliastro/iod/izzo.py#L105-L113
has to change and we probably should add a exception if no solution is feasible.
Generators
Pros of generators:
- Efficient
- Lazy
Con: If I want all solutions, I have to do list(sols)
first. As a result, direct printing does not work as expected (see for instance print(range(4))
).
Actually, this is related to the fact that Lambert transfers should be Maneuver
objects (see https://github.com/poliastro/poliastro/issues/114). Instead of returning a sequence of (va, vb)
pairs I should actually return a sequence of possible Maneuver
. Anyway, the underlying ideas are the same and I can think about this later.