Hierarchical parameter plot fix #1106
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
If, in any case (because some inner optimization failed, simulation failed, or some other reason) and there is a`optimize_result`dictionary in `result.optmize_result.list` which doesn't contain the `INNER_PARAMETERS` key and its item, the parameter plot would fail at line 381. This is a fix to make it robust to this. If any of the `optimize_result` objects has a `INNER_PARAMETER` key, then the other ones will be filled with NaN values, and plotted.
Doresic
requested review from
plakrisenko,
stephanmg,
PaulJonasJost,
yannikschaelte,
dilpath,
dweindl,
FFroehlich and
a team
as code owners
|
LGTM. |
yannikschaelte
approved these changes
Jul 28, 2023
stephanmg
approved these changes
Jul 28, 2023
dilpath
approved these changes
Jul 28, 2023
Codecov Report
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more. @@ Coverage Diff @@
## develop #1106 +/- ##
===========================================
- Coverage 88.16% 84.83% -3.34%
===========================================
Files 79 146 +67
Lines 5257 11419 +6162
===========================================
+ Hits 4635 9687 +5052
- Misses 622 1732 +1110
|
dweindl
approved these changes
Jul 28, 2023
Co-authored-by: Dilan Pathirana <59329744+dilpath@users.noreply.github.com>
Co-authored-by: Daniel Weindl <dweindl@users.noreply.github.com>
Testing the quality check
Merged
PaulJonasJost
added a commit
that referenced
this pull request
Oct 2, 2023
* Use cloudpickle for pickling dynesty sampler (#1094) Cloudpickle handles ABC-derived classes properly, whereas the latest dill version on pypi (0.3.6) does not. Cloudpickle is already installed with pypesto, so there seems to be no need to get dill on top. * Restrict fval magnitude in waterfall with order_by_id (#1090) * restrict waterfall with order_by_id to fvals < 1e100 * add constant for parameters/waterfall plot maximum value * Add startpoint_method to Problem (#1093) It's not intuitive that `x_guesses` is part of `Problem`, but `startpoint_method`s are handled separately. See also discussion in #1017. This patch * adds `startpoint_method` to `Problem` * during PEtab import, sets `Problem.startpoint_method` based on the PEtab problem * deprecates `startpoint_method` arguments where also `Problem` is passed Closes #1035 * Small initialize fix (#1095) Censoring bounds should not be reinitialized when calculators are reinitialized. * Added Falco et al to bib. (#1100) * fix storage (#1099) * Hierarchical parameter plot fix (#1106) * Fix parameter plot If, in any case (because some inner optimization failed, simulation failed, or some other reason) and there is a`optimize_result`dictionary in `result.optmize_result.list` which doesn't contain the `INNER_PARAMETERS` key and its item, the parameter plot would fail at line 381. This is a fix to make it robust to this. If any of the `optimize_result` objects has a `INNER_PARAMETER` key, then the other ones will be filled with NaN values, and plotted. * Dilan review change Co-authored-by: Dilan Pathirana <59329744+dilpath@users.noreply.github.com> * Update pypesto/visualize/parameters.py Co-authored-by: Daniel Weindl <dweindl@users.noreply.github.com> * Add comments * Add comment Testing the quality check * Fix quality check error --------- Co-authored-by: Dilan Pathirana <59329744+dilpath@users.noreply.github.com> Co-authored-by: Daniel Weindl <dweindl@users.noreply.github.com> * Fix startpoint sampling for hierarchical optimization (#1105) * Fix startpoint sampling for hierarchical optimization See #1096 * fixed pars * fixup --------- Co-authored-by: Doresic <85789271+Doresic@users.noreply.github.com> * PetabJL integration (#1089) * SingleCore engine works, multicore has problems with pickling even though manual pickling works * intermediate commit * Working tests. * small changes needs verification * created base test to check for integration into python. * Added julia module file for test * Added test for objective function evaluation * Added values for evaluation * Added installation of packages to Workflow. * Readded the Julia files that for some reason were deleted. * Removed PetabJL from default pypesto.petab import * Temporariy added amici as dependency. Needs to be taken care of appropriately. * Moved Importer to objective/julia * Deactivated precompilation in test * Integrated suggestions * add comments * add sundials to ci --------- Co-authored-by: Yannik Schälte <31767307+yannikschaelte@users.noreply.github.com> * Fix #1108 (#1109) * enable setting the y limits of a waterfall plot by using the y_limits argument * use default y limits for zoomed in starts * fix #1108 Co-authored-by: Paul Jonas Jost <70631928+PaulJonasJost@users.noreply.github.com> * SacessOptimizer: retry reading, delay deleting (#1110) * delete temporary files only after *all* were read successfully * retry reading temp file upon error to work around potential filesystem latency issues * SacessOptimizer: Fix logging with multiprocessing (#1112) * SacessOptimizer: Fix logging with multiprocessing * Fix: `ValueError: setting an array element with a sequence.` * Other platform tests (#1113) * blind attempt mac * brew installs * try python 3.11 numba * try windows * fix cache * stuff * stuff * stuff * stuff * minimal test * add basic workflow test * Update .github/workflows/ci.yml Co-authored-by: Daniel Weindl <dweindl@users.noreply.github.com> * close figures --------- Co-authored-by: Daniel Weindl <dweindl@users.noreply.github.com> * SacessOptimizer: tmpdir option (#1115) SacessOptimizer: * Add option to pass a custom directory for temporary files * Make the default temporary file paths unique, to avoid name collisions when running multiple SacessOptimizer instances from within the same working directory * Notebook on differences (#1098) * Notebook on differences * simplify some parts; add some more details and visualizations * minor edit * add workflow comparison to docs * integrated suggestions * integrated suggestions 2 * fixup * removed personal info. changed nlop objective appropriatly --------- Co-authored-by: Yannik Schälte <31767307+yannikschaelte@users.noreply.github.com> Co-authored-by: Yannik Schaelte <yannik.schaelte@gmail.com> * Documentation fixes (#1120) * document hierarchical optimization sources; add aesara+jax to objective docs * document origin of the mh + pt samplers * fix typo * fix typo * Update pypesto/sample/parallel_tempering.py Typo * streamline doc deps * update setup req versions * try cyipopt fix * test * tesT * test * fix test_visualize::test_optimization_scatter_with_x_None * test * fixup --------- Co-authored-by: Paul Jonas Jost <70631928+PaulJonasJost@users.noreply.github.com> * update changelog+version * Changed Codeowners and contact to accomodate changes in personell (#1123) * Updated version and Changelog (#1122) * Updated version and Changelog * Updated Changelog * Update CHANGELOG.rst --------- Co-authored-by: Daniel Weindl <dweindl@users.noreply.github.com> Co-authored-by: Maren Philipps <55318391+m-philipps@users.noreply.github.com> Co-authored-by: Doresic <85789271+Doresic@users.noreply.github.com> Co-authored-by: Polina Lakrisenko <p.lakrisenko@gmail.com> Co-authored-by: Dilan Pathirana <59329744+dilpath@users.noreply.github.com> Co-authored-by: Yannik Schälte <31767307+yannikschaelte@users.noreply.github.com> Co-authored-by: Yannik Schaelte <yannik.schaelte@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The issue was that the parameter plot was not showing hierarchical parameters in some cases.
If, in any case (because some inner optimization failed, simulation failed, or some other reason) there is an
optimize_resultdictionary inresult.optmize_result.listwhich doesn't contain theINNER_PARAMETERSkey and its item, the parameter plot would fail at line 381.This is a fix to make it robust to this. If any of the
optimize_resultobjects have anINNER_PARAMETERkey, then the other ones will be filled with NaN values and plotted.