Bug fix in IPOPT wrapper #470
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #470 +/- ##
==========================================
- Coverage 86.73% 86.69% -0.05%
==========================================
Files 24 24
Lines 3460 3464 +4
==========================================
+ Hits 3001 3003 +2
- Misses 459 461 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| # IPOPT calls objective and constraints separately, so we find two call counters and append iter_dict to both counters. | ||
| call_counter_1 = self.hist._searchCallCounter(self.cache["x"]) | ||
| call_counter_2 = self.hist._searchCallCounter(self.cache["x"], last=call_counter_1 - 1) | ||
| call_counter_1 = self.hist._searchCallCounter(self.optProb._mapXtoUser(self.cache["x"])) |
There was a problem hiding this comment.
Actually, I wonder if we should have some checks in case even with scaling we fail to find the call counter. In such cases we should be able to continue gracefully without raising errors
There was a problem hiding this comment.
I am not sure if from a user perspective it would be better to continue with a warning or exit, but I agree we should probably add a check (try/except?) to make the error more understandable?
There was a problem hiding this comment.
I don't think we should exit, I think the correct behaviour is to potentially warn, and then continue without saving these additional variables. I don't think an issue in saving some extra parameters during the optimization should kill the optimization,. it is very important that we continue the optimization loop wherever possible.
There was a problem hiding this comment.
Modified here to keep going if the matching call counter cannot be found and print a warning.
Purpose
This PR fixes a bug in the IPOPT wrapper #459 introduced. Closes #469 .
I messed up with the variable scaling in the previous PR and overlooked it because we didn't have an IPOPT test with scaling. Now I modified one of the HS071 tests to run IPOPT with scaling, which would've caught this bug.
Expected time until merged
Urgent (and should make a patch release)
Type of change
Testing
Checklist
ruff checkandruff formatto make sure the Python code adheres to PEP-8 and is consistently formattedfprettifyor C/C++ code withclang-formatas applicable