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.

Sign up for GitHub

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

Populate inform dict for all optimizers #394

Merged
merged 6 commits into from
Apr 24, 2024
Merged

Populate inform dict for all optimizers #394

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
3f8875b
populate inform dict
ewu63 Apr 23, 2024
a91a482
bugfix version bump
marcomangano Apr 23, 2024
44a9ed2
revert version bump
marcomangano Apr 23, 2024
864c561
use empty string instead of None
ewu63 Apr 23, 2024
6f7a181
test print(sol)
ewu63 Apr 23, 2024
cec9046
Merge branch 'main' into bugfix/nsga2
marcomangano Apr 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions pyoptsparse/pyALPSO/pyALPSO.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
pyALPSO - A pyOptSparse interface to ALPSO
work with sparse optimization problems.
"""

# Standard Python modules
import datetime
import time
Expand Down Expand Up @@ -191,9 +192,7 @@ def objconfunc(x):
self.optProb.comm.bcast(-1, root=0)

# Store Results
sol_inform = {}
# sol_inform['value'] = inform
# sol_inform['text'] = self.informs[inform[0]]
sol_inform = {"value": "", "text": ""}
Copy link
Contributor

@swryan swryan Apr 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was "value" intentionally set to a string value in these optimizers?

As far as I can tell, this was always meant to be an integer return code, and we have logic that expects that to be the case:

File "/home/swryan/dev/OpenMDAO/openmdao/drivers/pyoptsparse_driver.py", line 680, in run
    if exit_status > 2:
       ^^^^^^^^^^^^^^^
TypeError: '>' not supported between instances of 'str' and 'int'

Just wondering what the circumstances would be where we should expect this value to be a string...

Copy link
Contributor

@swryan swryan Apr 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm.. I see that we were checking if "value" was in the dict.. and if it is there we expect it to be an int....

So we were handling the case for the optimizers that didn't populate it... I guess we should consider an empty string value to be an indication that the optimizer doesn't provide the value...

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I see, I guess we need to agree on a convention for optimizers that don't have any informs. I'm happy to do whatever you guys want, which could be

  • None
  • empty string (current)
  • some default inform that just says optimization completed

In the longer term, we should probably make dataclasses and type annotate these better.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In retrospect this PR is probably not the best implementation, and was done purely to make the solution object printing work without changing the logic there...

Copy link
Contributor

@swryan swryan Apr 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, it may be that just updating pyOpt_solution.py to only print values for those keys if they exist might a preferable solution, keeping the semantics/typing a bit more rigorous, but it's easy enough for us to handle the empty string (which we will do). Also agree with your long term thoughts.


# Create the optimization solution
sol = self._createSolution(optTime, sol_inform, opt_f, opt_x)
Expand Down
5 changes: 2 additions & 3 deletions pyoptsparse/pyCONMIN/pyCONMIN.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
pyCONMIN - A variation of the pyCONMIN wrapper specificially designed to
work with sparse optimization problems.
"""

# Standard Python modules
import datetime
import os
Expand Down Expand Up @@ -240,9 +241,7 @@ def cnmngrad(n1, n2, x, f, g, ct, df, a, ic, nac):
self.optProb.comm.bcast(-1, root=0)

# Store Results
sol_inform = {}
# sol_inform['value'] = inform
# sol_inform['text'] = self.informs[inform[0]]
sol_inform = {"value": "", "text": ""}

# Create the optimization solution
sol = self._createSolution(optTime, sol_inform, ff, xs)
Expand Down
3 changes: 2 additions & 1 deletion pyoptsparse/pyNSGA2/pyNSGA2.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
pyNSGA2 - A variation of the pyNSGA2 wrapper specificially designed to
work with sparse optimization problems.
"""

# Standard Python modules
import os
import time
Expand Down Expand Up @@ -180,7 +181,7 @@ def objconfunc(nreal, nobj, ncon, x, f, g):
self.optProb.comm.bcast(-1, root=0)

# Store Results
sol_inform = {}
sol_inform = {"value": "", "text": ""}

xstar = [0.0] * n
for i in range(n):
Expand Down
4 changes: 1 addition & 3 deletions pyoptsparse/pyParOpt/ParOpt.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,9 +235,7 @@ def evalObjConGradient(self, x, g, A):
# Create the optimization solution. Note that the signs on the multipliers
# are switch since ParOpt uses a formulation with c(x) >= 0, while pyOpt
# uses g(x) = -c(x) <= 0. Therefore the multipliers are reversed.
sol_inform = {}
sol_inform["value"] = None
sol_inform["text"] = None
sol_inform = {"value": "", "text": ""}

# If number of constraints is zero, ParOpt returns z as None.
# Thus if there is no constraints, should pass an empty list
Expand Down
3 changes: 3 additions & 0 deletions tests/testing_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,9 @@ def assert_solution_allclose(self, sol, tol, partial_x=False):
):
assert_dict_allclose(sol.lambdaStar, self.lambdaStar[self.sol_index], atol=tol, rtol=tol)

# test printing solution
print(sol)

def assert_inform_equal(self, sol, optInform=None):
"""
Check that the optInform stored in the Solution object is as expected.
Expand Down
Loading