Added restartDict to snstop
Purpose
I added an option to save restartDict after each major iteration. This is helpful when you want to restart optimizations after they crash unexpectedly or run out of time. In the latter case, this avoids the guesswork of setting a conservative time limit. I also added an option that allows restartDict to be passed to snstop_handle if the user wants to access this after every major iteration.
Expected time until merged
1-2 weeks
Type of change
- [ ] Bugfix (non-breaking change which fixes an issue)
- [x] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (non-backwards-compatible fix or feature)
- [ ] Code style update (formatting, renaming)
- [ ] Refactoring (no functional changes, no API changes)
- [ ] Documentation update
- [ ] Maintenance update
- [ ] Other (please describe)
Testing
I added a test that writes restartDict using snstop and uses the dictionary to restart the optimization. I also tested this with my aerodynamic shape optimization cases.
Checklist
- [x] I have run
flake8andblackto make sure the Python code adheres to PEP-8 and is consistently formatted - [ ] I have formatted the Fortran code with
fprettifyor C/C++ code withclang-formatas applicable - [x] I have run unit and regression tests which pass locally with my changes
- [x] I have added new tests that prove my fix is effective or that my feature works
- [x] I have added necessary documentation
Codecov Report
Attention: Patch coverage is 83.33333% with 2 lines in your changes missing coverage. Please review.
Project coverage is 74.92%. Comparing base (
d96a398) to head (19a3fd7). Report is 2 commits behind head on main.
| Files with missing lines | Patch % | Lines |
|---|---|---|
| pyoptsparse/pySNOPT/pySNOPT.py | 81.81% | 2 Missing :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## main #404 +/- ##
==========================================
+ Coverage 74.88% 74.92% +0.03%
==========================================
Files 22 22
Lines 3325 3334 +9
==========================================
+ Hits 2490 2498 +8
- Misses 835 836 +1
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Maybe also do a version bump here (minor?)
Thanks for the comments. It would be nice for the function to be more flexible. I'm not sure how returning values would work though because the user does not call
_snstop. It seems like the dictionaries have to be passed in.
The way I'm thinking is the same as how we implemented Save major iteration variables. Basically, pass in a list of what you want, and snstop will be called with those args, matching the order. By default we can always pass iterDict.
Another thought - we can also make the "save work arrays at every iter" an option, instead of asking the user to make the snstop function. We can add an option save work arrays which by default is False or None, but you can set a path and then the built-in snstop function can store these (in some preferred format). This will standardize the restarts.
This is ready for another review. I implemented @ewu63's suggestions so this is more useable and is no longer a breaking change.
So I forgot that we made it not a breaking change, meaning we should've bumped patch and not minor version... oh well a bit late now, I'd rather not leave any commits on main with incorrect version numbers.
So I forgot that we made it not a breaking change, meaning we should've bumped patch and not minor version... oh well a bit late now, I'd rather not leave any commits on
mainwith incorrect version numbers.
I bumped the minor version because this is technically a new feature, I think we got a bit loose with version numbers tbh. Not a big deal, but we can be a bit more strict in the future
So I forgot that we made it not a breaking change, meaning we should've bumped patch and not minor version... oh well a bit late now, I'd rather not leave any commits on
mainwith incorrect version numbers.I bumped the minor version because this is technically a new feature, I think we got a bit loose with version numbers tbh. Not a big deal, but we can be a bit more strict in the future
In general I have shied away from semver when I was in the lab, and instead stuck largely to Numpy's versioning scheme, for the reasons outlined there. There is nothing inherently wrong with bumping the minor version frequently, but it should carry some weight behind it - otherwise we quickly move to v2.52.0 without actual substantial changes to the software.