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

Updated phasing/_lambert to pygmo2 #142

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

albertoibm
Copy link
Contributor

While correcting errors in the lint test, it was evident that the file phasing/_lambert needed to be updated to use pygmo 2.

@darioizzo
Copy link
Member

darioizzo commented Nov 20, 2020

Thanks ... update long needed :)

Can you merge master into this PR and see if the CI passes?

@darioizzo
Copy link
Member

linting failed ... can you fix it?

@albertoibm
Copy link
Contributor Author

linting failed ... can you fix it?

yes but I have a question. It fails because it's trying to get f_dimension which I imagine existed in the base class. The dimensions in this case are the departing and arrival times, right? The code seems to allow for a 1 dimensional problem but I would have assumed the dimension is 2.

@darioizzo
Copy link
Member

I see now the UDP (user defined problem) is malformed. Note that the user problem class is not inheriting from any base anymore: in pygmo2 the syntax has changed considerably w.r.t. version 1.x ...

We have no longer base classes, so you need to write this UDP without relying on pygmo at all. It must be self standing, but respecting the pygmo UPD interface, see https://esa.github.io/pygmo2/tutorials/coding_udp_multi_objective.html for an example?

In your case you will need to add to the mandatory ones to the get_nobj method returning 2 in multiobjective case.

To access the problem dimension you will need to use the methods available in your class.

@albertoibm
Copy link
Contributor Author

Yes, that I understood but it wasn't clear to me how to determine the value of f_dimension. After reading the paper and again checking the code I think commit b885ba1 would be correct. The linting passed, but do check the changes.

@darioizzo
Copy link
Member

darioizzo commented Nov 23, 2020

Thanks for the modifications .. going in the right direction,

You still need something like:

def get_nobj(self):
    return self.f_dimension

... please test by constructing a pygmo problem with it, like:

import pygmo
udp = lambert_metric(....)
prob = pygmo.problem(udp)
print(prob)
pop  =pygmo.population(prob, 20)

and check all is good.

Also if you could update the docstring:

The result is a pygmo multi-objective problem that can be solved efficiently by MO optimization algorithms

to

The result is a class that can be used as is or to construct a pygmo problem that can be solved efficiently by the available optimization algorithms

it would be great!

@albertoibm
Copy link
Contributor Author

albertoibm commented Dec 9, 2020

Sorry for the long delay. I did all the changes and tried testing the way you suggest here and when doing


>>> udp = pykep.phasing.lambert_metric()
>>> prob = pygmo.problem(udp)

I get this error

ValueError: array has incorrect number of dimensions: 0; expected 1
By any chance do you know it?
I'll keep digging into pygmo to try and find out what it is but as for now I don't understand it.

@darioizzo darioizzo marked this pull request as ready for review November 15, 2022 19:55
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.

2 participants