Repository navigation
Conversation
|
Tests fail because the docker images still use I am now wondering if we should just move |
|
I will probably keep it in our docker images anyway because it's nice to have. But I agree it'd be nice for pyoptsparse to handle. Right now we have a chicken and egg situation where we've merged a PR updating the unopy version in the docker images, but those new images won't be published until our docker CI passes, which won't happen until this PR is merged. Handling the unopy installation in the pyoptsparse CI would fix this.
Not super familiar with the precedent in other pyOptSparse wrappers, but I think we should only be storing default options in pyOptSparse if we are deliberately overriding that default, or we alter the behaviour of the option (e.g the |
|
Putting myself on the spot because I triggered this whole roadblock when I first added UNO to docker without realizing that If we do this, then it is also much easier to maintain the current set of default options - whatever they may be. |
The Uno options in pyOptSparse are already quite minimal 😅 But Uno is under active development and the option set changes pretty much daily. That said, the tolerances that changed from Uno v2.9.0 to v2.9.1 shouldn't change any more (sorry, I should have published a minor release because of the breaking changes). |
|
If I look here, there are only 3 options that we currently handle differently than Uno:
IMO |
--om3-petsc swaps OM3's coupled-group DirectSolver for PETScDirectSolver using the same sparse_solver_name as --linear-solver, so the petsc-* linear solver choices can be cross-checked against OM3 instead of only compared within om4.
Also pins unopy below 0.4.15: that release dropped loose_primal_tolerance/loose_dual_tolerance/loose_tolerance_iteration_threshold, which pyoptsparse's pyUno wrapper still sets unconditionally, breaking every UNO run with KeyError('loose_primal_tolerance does not exist'). Fixed upstream in mdolab/pyoptsparse#501 but not yet released; pin removed once a pyoptsparse release includes that fix. Verified --opt pyoptsparse-uno converges on the 2x5 mesh after pinning.
unopy 0.4.17 supports setting the preset with set_option("preset", ...),
so the special-case set_preset() call is no longer needed. Bump the
minimum unopy version to 0.4.17 and change the default preset to "auto"
to match unopy.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A-CGray
left a comment
There was a problem hiding this comment.
I've updated the uno wrapper to set the preset through set_option rather than set_preset so it doesn't require any special handling. This relies on a bugfix in unopy 0.4.17.
@cvanaret unopy currently doesn't have any API for retrieving option values (either the currently set or default values). It seems like it does exist in the C++ API, if that could be exposed in unopy then we could get rid of most of the hardcoded options/defaults in _getDefaultOptions and automatically reflect the up to date options, even as they change between releases.
Purpose
This PR updates the pyUno options. The tolerances associated with loose tolerances were changed/renamed in unopy v0.4.15/Uno v2.9.1.
Expected time until merged
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