PLT - add visualization of objective as function of `stop_val` #479

Badr-MOUFAD · 2022-11-09T20:45:41Z

This adds a toggle to enable visualizing the objectives as function stop_val, namely n_iter.

Link to a dummy benchmark to preview the feature. (credit to @mathurinm for the idea of the benchmark).
A link to the benchmark repo to reproduce.

PR related to #287

…iteration-plot

Badr-MOUFAD · 2022-11-12T14:34:05Z

any thoughts @mathurinm?

mathurinm · 2022-11-15T15:29:46Z

Thanks for having a look @Badr-MOUFAD ! I can't see anything for the other objective, is it normal ?
https://hal.archives-ouvertes.fr/hal-01941152v2/document

How does this handle combination of solvers that use iteration stop_val and tol stop val ?

I'd try to find an explicit name for the toggle button, like "plot a function of stopping criterion" but shorter... not easy :)

Badr-MOUFAD · 2022-11-15T16:02:44Z

Thanks for having a look @Badr-MOUFAD ! I can't see anything for the other objective, is it normal ?

@mathurinm, the values of the objective are negative and hence can't be visualized using a log-y scale.
Try linear scale or in the sidebar set the objective to Affine[with_exp=True]

Badr-MOUFAD · 2022-11-16T10:32:15Z

How does this handle combination of solvers that use iteration stop_val and tol stop val ?

Actually, it doesn't handle this case and I don't see how we can achieve that (also, I don't find a column in the .parquet file that informs about the type of stop criterion).

IMO, plotting as a function of stop_crit would make sense if all solvers use the same stop_crit and that stop_crit has the same scale for all solvers (e.g. a solver that converges in 10 iterations, yet under the hood runs several epochs, versus a solver that converges in 1000 iterations).

jeandut · 2022-12-29T15:23:23Z

Some minor comments after @mathurinm's invitation (to take with a grain of salt):

Stopping Criterion is not intuitive for the name of such a toggle, an alternative name would at least include "X-axis" somewhere
Stop_value is also unintuitive #iterations is better
I would avoid scientific notation for the x-axis in the case of iterations and use the integer prior over the indices to have a better visualization (as in matplotlib MaxNLocator(integer=True))
The default scale for a plot function of iterations should be linear

(Not linked to this PR, but with benchopt the kind of ZigZag pattern that can be observed in the current result plot is not rare and a little bit puzzling)

tomMoral · 2023-01-02T19:57:59Z

Actually, I am not sure we want to add a switch.
I would be more in favor of a solution to have a selector that allows to put any metric as x and any as y? (keeping the default to time and first objective value).
This would make the UI very explicit that one is choosing the axis value.
And it would also solve extra use cases, where people want to look for pareto front with multiple objectives.

Also, this would allow to separate iterations/tolerance, which I think is a pain to handle right now (and probably why it is called stop_value in the PR).

WDYT?

jeandut · 2023-02-23T10:48:31Z

I would like to reiterate that the completion of this PR would be extremely valuable for a lot of us in the community.
I side with @tomMoral on the selector for x instead of a toggle. It would allow to very easily add more resampling options in the long-term.

I also remain adamant that at least on the display html, names should reflect what the user is interested in and not variable names from the codebase i.e. iterations/tolerance and not StoppingCriterion or stop_val.

It would be really great as well if we can have integer ticks if x == iterations.

Cheers.

tomMoral · 2023-02-23T19:20:09Z

As discussed with @mathurinm IRL, I propose the following solution to move on with this PR:

We add a drop-down menu with ['iteration', 'tolerance'] on the page.
In the solver_data[solver]['scatter'] dictionary, we add a value 'strategy' in iteration, tolerance.
Depending on the drop-down menu, we display only the solver with the strategy that matches the one selected in the drop-down menu.

I would say this looks like a good first step, without adding too much complexity? WDYT @Badr-MOUFAD ?

codecov-commenter · 2023-02-23T19:26:12Z

Codecov Report

Merging #479 (0e1b40f) into main (1487639) will decrease coverage by 0.07%.
The diff coverage is 33.33%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##             main     #479      +/-   ##
==========================================
- Coverage   54.64%   54.58%   -0.07%     
==========================================
  Files          43       43              
  Lines        2796     2805       +9     
  Branches      511      514       +3     
==========================================
+ Hits         1528     1531       +3     
- Misses       1153     1159       +6     
  Partials      115      115

Badr-MOUFAD · 2023-02-23T21:45:45Z

Yes, it sounds good to me.

Badr-MOUFAD · 2023-02-28T17:05:48Z

@tomMoral, I have a little concern about this point

In the solver_data[solver]['scatter'] dictionary, we add a value 'strategy' in iteration, tolerance.

In fact, we don't store the strategy in the data frame of results and hence we cannot determine the stopping strategy at the plotting stage: generate_html.py.

One workaround is to add the stopping strategy attribute in meta when running the benchmark. That way the stopping_startegy would be available in the final data frame

benchopt/benchopt/runner.py

Lines 187 to 192 in 1487639

    
           meta = dict( 
        
               objective_name=str(objective), 
        
               solver_name=str(solver), 
        
               data_name=str(dataset), 
        
               idx_rep=rep, 
        
           )

WDYT @tomMoral? Do you see another workaround?

tomMoral · 2023-03-01T00:09:53Z

WDYT @tomMoral? Do you see another workaround?

I would have suggested the same so perfect :) As the dataframe is stored as a parquet (thus compressed), the impact on the output file's size should be minim so works for me.

Badr-MOUFAD · 2023-03-02T16:17:59Z

With the latter remarks being taken into account here is a preview of the feature.

The xaxis type drop menu acts also as a filter: only the solver with a stopping strategy matching xaxis type will be shown.

@tomMoral, @jeandut I am open to suggestions.

benchopt/plotting/generate_html.py

tomMoral

LGTM! a few comments but I really like the result, it looks very smooth! thanks a lot @Badr-MOUFAD

the two things to fix:

callback strategy should be mapped to iteration
the drop down menu should contain only values present in the dataframe

tomMoral · 2023-03-02T18:31:34Z

benchopt/plotting/html/templates/result.mako.html

+                                    <option value="time">Time</option>
+                                    <option value="iteration">Iteration</option>
+                                    <option value="tolerance">Tolerance</option>


can this be parametrized to avoid putting iteration or tolerance if it does not exists in the data?
Actually, all these values should come from the ['time'] + list(df['stopping_strategy'].unique())

I opted for dynamically creating the options depending on the selected dataset and objective columns. In fact, some solvers might be skipped in some cases.

Any thoughts?

benchopt/runner.py

benchopt/plotting/html/static/result.js

jeandut · 2023-03-03T08:50:03Z

With the latter remarks being taken into account here is a preview of the feature.

The xaxis type drop menu acts also as a filter: only the solver with a stopping strategy matching xaxis type will be shown.

@tomMoral, @jeandut I am open to suggestions.

Very cool work it looks very good !
A few disorganised comments on the preview:

I would remove the word type after X-axis.
Tolerance axis does not seem to work in this example
would love to have integer tick labels when switching to iterations but this is a detail

Overall it's very nice !

Badr-MOUFAD · 2023-03-03T14:39:58Z

Thanks, @jeandut for the feedback! Here is the updated preview of the feature.

tomMoral · 2023-03-04T00:02:12Z

Could not check on my computer right now but any idea why the x-axis is not visible on mobile?

I will review it tomorrow but the changes look good and good idea to create the drop down menu dynamically, it is better to only have non empty option 😁

Badr-MOUFAD · 2023-03-04T12:39:10Z

Could not check on my computer right now but any idea why the x-axis is not visible on mobile?

I have just become aware that the responsiveness (mobile support) was implemented by duplicating the sidebar's content and styling it separately.

benchopt/benchopt/plotting/html/templates/result.mako.html

Lines 22 to 25 in d520e74

    
           <body> 
        
               <!-- Two main columns container --> 
        
               <div class="flex"> 
        
                   <!-- Left column on big screen (plot selectors) -->

benchopt/benchopt/plotting/html/templates/result.mako.html

Lines 97 to 99 in d520e74

    
           </div> 
        
           <!-- Right column on big screen and main column on mobile --> 
        
           <div class="w-full flex flex-col overflow-scroll">

Hence to fix #479 (comment), we should make the same changes we did for the sidebar of the big screen to the one on the mobile screen.

Responsiveness is usually implemented through breakpoints that specify the style to be applied according to the screen size.
A follow-up TODO of this PR is to remove the content duplication and implement properly the responsiveness.

P.S. I updated the feature preview

jeandut · 2023-03-04T15:07:53Z

Thanks, @jeandut for the feedback! Here is the updated preview of the feature.

Awesome !!!

jeandut · 2023-03-04T15:14:09Z

Although there might be a bug see values in iteration axis when converting to log-log ?:
1-2-5-10-2-5-100-2-5-1000 etc.

tomMoral

Ok, it looks good! Just a small fix for the xticks and 2 nitpicks and we are good to go!
Thx @Badr-MOUFAD !

benchopt/plotting/generate_html.py

benchopt/plotting/html/static/result.js

benchopt/runner.py

tomMoral · 2023-03-04T15:45:38Z

Could not check on my computer right now but any idea why the x-axis is not visible on mobile?

I have just become aware that the responsiveness (mobile support) was implemented by duplicating the sidebar's content and styling it separately.

benchopt/benchopt/plotting/html/templates/result.mako.html

Lines 22 to 25 in d520e74

<body>



<div class="flex">



benchopt/benchopt/plotting/html/templates/result.mako.html

Lines 97 to 99 in d520e74

</div>



<div class="w-full flex flex-col overflow-scroll">

Hence to fix #479 (comment), we should make the same changes we did for the sidebar of the big screen to the one on the mobile screen.

Responsiveness is usually implemented through breakpoints that specify the style to be applied according to the screen size. A follow-up TODO of this PR is to remove the content duplication and implement properly the responsiveness.

P.S. I updated the feature preview

Thanks a lot for the explanation, indeed it would be much better to implement it with the right synthax.

Badr-MOUFAD · 2023-03-04T15:55:09Z

Although there might be a bug see values in iteration axis when converting to log-log ?: 1-2-5-10-2-5-100-2-5-1000 etc.

Looking at Plotly log scale doc, I don't think it is a bug.
Looks fine to me. Do you think it might be misleading @jeandut?

jeandut · 2023-03-04T23:32:34Z

Although there might be a bug see values in iteration axis when converting to log-log ?: 1-2-5-10-2-5-100-2-5-1000 etc.

Looking at Plotly log scale doc, I don't think it is a bug. Looks fine to me. Do you think it might be misleading @jeandut?

You are totally right. The tick values do make sense somehow on log-scale. Looks like it's ready to be merged ! Great work !

tomMoral · 2023-03-05T05:48:01Z

merged, thanks a lot for making this feature @Badr-MOUFAD !

Badr-MOUFAD added 4 commits November 9, 2022 10:48

POC plot iteration

ccc9df5

add stop crit in dict result

96655ab

format xaxis title

0f975df

Merge branch 'main' of https://github.com/benchopt/benchopt into add-…

65a95cb

…iteration-plot

tomMoral mentioned this pull request Nov 21, 2022

PLT add plot with iterations #287

Closed

mathurinm mentioned this pull request Dec 29, 2022

[RFC] Changing time axis to be something else (iterations/comm. rounds/epochs) in benchopt reporting #526

Closed

Merge branch 'main' into add-iteration-plot

d673c7b

Badr-MOUFAD force-pushed the add-iteration-plot branch 2 times, most recently from 030ea2e to d673c7b Compare February 28, 2023 16:37

Badr-MOUFAD added 5 commits March 2, 2023 15:44

add stopping strategy to df results

5cb6b98

add form to select xaxis type

effb2d3

add stopping_strategy to the shipped result df

82286dd

render plots considering xaxis type

df43d87

fix pandas key error

09cd0cb

Badr-MOUFAD commented Mar 2, 2023

View reviewed changes

benchopt/plotting/generate_html.py Show resolved Hide resolved

tomMoral reviewed Mar 2, 2023

View reviewed changes

Badr-MOUFAD added 5 commits March 3, 2023 10:30

handle callback sto strategy

e5a92d8

add sanity check for stop strategy

2ec86b4

dynamic options in selection

1f72e8e

skip quantiles for non time

47253ba

xaxis label formatting

6d313ad

Badr-MOUFAD requested a review from tomMoral March 3, 2023 14:40

support for mobile screen size

0e1b40f

tomMoral approved these changes Mar 4, 2023

View reviewed changes

benchopt/plotting/generate_html.py Outdated Show resolved Hide resolved

benchopt/plotting/html/static/result.js Outdated Show resolved Hide resolved

benchopt/runner.py Outdated Show resolved Hide resolved

tomMoral remarks

3602c5b

tomMoral merged commit 6b67c3c into benchopt:main Mar 5, 2023

Badr-MOUFAD deleted the add-iteration-plot branch March 5, 2023 09:43

mathurinm mentioned this pull request Mar 9, 2023

Querying stopping_strategy from #479 breaks rendering of older benchmarks #545

Closed

Badr-MOUFAD mentioned this pull request Mar 9, 2023

PLT - preserve plotting for benchmark run in previous version #546

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PLT - add visualization of objective as function of `stop_val` #479

PLT - add visualization of objective as function of `stop_val` #479

Badr-MOUFAD commented Nov 9, 2022 •

edited

Badr-MOUFAD commented Nov 12, 2022

mathurinm commented Nov 15, 2022 •

edited

Badr-MOUFAD commented Nov 15, 2022

Badr-MOUFAD commented Nov 16, 2022

jeandut commented Dec 29, 2022

tomMoral commented Jan 2, 2023

jeandut commented Feb 23, 2023 •

edited

tomMoral commented Feb 23, 2023

codecov-commenter commented Feb 23, 2023 •

edited

Badr-MOUFAD commented Feb 23, 2023

Badr-MOUFAD commented Feb 28, 2023

tomMoral commented Mar 1, 2023

Badr-MOUFAD commented Mar 2, 2023 •

edited

tomMoral left a comment •

edited

tomMoral Mar 2, 2023

Badr-MOUFAD Mar 3, 2023

jeandut commented Mar 3, 2023 •

edited

Badr-MOUFAD commented Mar 3, 2023 •

edited

tomMoral commented Mar 4, 2023

Badr-MOUFAD commented Mar 4, 2023 •

edited

jeandut commented Mar 4, 2023

jeandut commented Mar 4, 2023 •

edited

tomMoral left a comment

tomMoral commented Mar 4, 2023

Badr-MOUFAD commented Mar 4, 2023

jeandut commented Mar 4, 2023

tomMoral commented Mar 5, 2023

PLT - add visualization of objective as function of stop_val #479

PLT - add visualization of objective as function of stop_val #479

Conversation

Badr-MOUFAD commented Nov 9, 2022 • edited

Badr-MOUFAD commented Nov 12, 2022

mathurinm commented Nov 15, 2022 • edited

Badr-MOUFAD commented Nov 15, 2022

Badr-MOUFAD commented Nov 16, 2022

jeandut commented Dec 29, 2022

tomMoral commented Jan 2, 2023

jeandut commented Feb 23, 2023 • edited

tomMoral commented Feb 23, 2023

codecov-commenter commented Feb 23, 2023 • edited

Codecov Report

Badr-MOUFAD commented Feb 23, 2023

Badr-MOUFAD commented Feb 28, 2023

tomMoral commented Mar 1, 2023

Badr-MOUFAD commented Mar 2, 2023 • edited

tomMoral left a comment • edited

Choose a reason for hiding this comment

tomMoral Mar 2, 2023

Choose a reason for hiding this comment

Badr-MOUFAD Mar 3, 2023

Choose a reason for hiding this comment

jeandut commented Mar 3, 2023 • edited

Badr-MOUFAD commented Mar 3, 2023 • edited

tomMoral commented Mar 4, 2023

Badr-MOUFAD commented Mar 4, 2023 • edited

jeandut commented Mar 4, 2023

jeandut commented Mar 4, 2023 • edited

tomMoral left a comment

Choose a reason for hiding this comment

tomMoral commented Mar 4, 2023

Badr-MOUFAD commented Mar 4, 2023

jeandut commented Mar 4, 2023

tomMoral commented Mar 5, 2023

PLT - add visualization of objective as function of `stop_val` #479

PLT - add visualization of objective as function of `stop_val` #479

Badr-MOUFAD commented Nov 9, 2022 •

edited

mathurinm commented Nov 15, 2022 •

edited

jeandut commented Feb 23, 2023 •

edited

codecov-commenter commented Feb 23, 2023 •

edited

Badr-MOUFAD commented Mar 2, 2023 •

edited

tomMoral left a comment •

edited

jeandut commented Mar 3, 2023 •

edited

Badr-MOUFAD commented Mar 3, 2023 •

edited

Badr-MOUFAD commented Mar 4, 2023 •

edited

jeandut commented Mar 4, 2023 •

edited