visualization of optimization result properties #486
Conversation
|
If anyone could suggest better function names would be nice :) |
Codecov Report
@@ Coverage Diff @@
## develop #486 +/- ##
===========================================
- Coverage 90.81% 90.70% -0.11%
===========================================
Files 70 71 +1
Lines 4027 4121 +94
===========================================
+ Hits 3657 3738 +81
- Misses 370 383 +13
Continue to review full report at Codecov.
|
|
Complexity increasing per file
==============================
- pypesto/visualize/optimization_stats.py 10
Clones added
============
- pypesto/visualize/optimization_stats.py 6
See the complete overview on Codacy |
| 'n_sres'] | ||
|
|
||
| if colors is None: | ||
| colors = assign_colors_for_list(len(properties_to_plot)) |
There was a problem hiding this comment.
Nice you re-use the assign_colors logic here. Well done!
|
|
||
|
|
||
| def optimization_run_properties_one_plot( | ||
| results: Result, |
There was a problem hiding this comment.
You might want to allow an iterable of results here as well, as the lower-level routine does ;)
There was a problem hiding this comment.
Would it be one plot per result then? In all other cases plots for different results are plotted in one figure. In this case, multiple optimization properties for different results all in one figure would be too much, I think. So I didn't allow it :)
| The plot axes. | ||
|
|
||
|
|
||
| Examples |
| """ | ||
|
|
||
| if properties_to_plot is None: | ||
| properties_to_plot = ['time', 'n_fval', 'n_grad', 'n_hess', 'n_res', |
There was a problem hiding this comment.
Do you really need that? If this routine is the lowlevel and only called via the high-level, you wouldn't need it, right?
So: do you want to expose this function to the outside world?
There was a problem hiding this comment.
I want to expose it. It can be used if one wants one figure per optimization property. Especially if there are multiple results. It will be, e.g. a figure with 'time' stats for all the results, a figure with 'n_fval' stats and so on
| properties_to_plot = ['time', 'n_fval', 'n_grad', 'n_hess', 'n_res', | ||
| 'n_sres'] | ||
|
|
||
| # TODO: check color and legend shape |
There was a problem hiding this comment.
Well, as the name says ;)
|
|
||
| # axes | ||
| if axes is None: | ||
| ncols = 2 if plot_type == 'both' else 1 |
There was a problem hiding this comment.
As I see this...
How will the logic for "both" work, if you specify an existing axes object?
| # loop over results | ||
| for j, result in enumerate(results): | ||
| if plot_type == 'both': | ||
| axes[0] = stats_lowlevel(result, opt_run_property, |
There was a problem hiding this comment.
ah, okay. So "both" will only really work with two subplots...
Would it make sense to double-check that beforehand? Not sure actually... If the user employs optional features, he/she should make sure to do that properly, maybe... So not sure whether extra double-checking is necessary...
| 'point. Stopping.') | ||
| if fval is not None: | ||
| self.fval = fval | ||
| else: | ||
| raise ValueError('Objective value fval not passed, but is a ' | ||
| 'manadatory input when creating a reference ' | ||
| 'mandatory input when creating a reference ' |
There was a problem hiding this comment.
thanks for correcting my typos! 😄
* Doc: Add 'scope' section and illustration of PEtab components * Image to figure, add caption, update references * fixup
#379