Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DOC - Enhance installation, running and visualizing benchmarks #629

Merged
merged 48 commits into from Jul 28, 2023

Conversation

Badr-MOUFAD
Copy link
Member

@Badr-MOUFAD Badr-MOUFAD commented Jul 21, 2023

Followup of #619. It brings the following changes

  • reformulate get started page
  • add run benchmark page
  • add visualize benchmark page

Checks before merging PR

  • added documentation for any new feature
  • added unit test
  • edited the what's new

@Badr-MOUFAD Badr-MOUFAD changed the title DOC - Enhance installation and running benchmarks DOC - Enhance installation, running and visualizing benchmarks Jul 21, 2023
@codecov-commenter
Copy link

codecov-commenter commented Jul 21, 2023

Codecov Report

Merging #629 (f28a959) into main (07793b6) will not change coverage.
Report is 1 commits behind head on main.
The diff coverage is n/a.

❗ 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           @@
##             main     #629   +/-   ##
=======================================
  Coverage   55.73%   55.73%           
=======================================
  Files          44       44           
  Lines        2869     2869           
  Branches      528      528           
=======================================
  Hits         1599     1599           
  Misses       1153     1153           
  Partials      117      117           

doc/benchmark_workflow/visualize_benchmark.rst Outdated Show resolved Hide resolved
doc/benchmark_workflow/visualize_benchmark.rst Outdated Show resolved Hide resolved
doc/benchmark_workflow/visualize_benchmark.rst Outdated Show resolved Hide resolved
doc/benchmark_workflow/visualize_benchmark.rst Outdated Show resolved Hide resolved
doc/get_started.rst Outdated Show resolved Hide resolved

With benchopt being installed, you get access to the :ref:`Command Line Interface (CLI) <cli_ref>`,
which enables simple and easy manipulation of benchmarks just from the terminal.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
which enables simple and easy manipulation of benchmarks just from the terminal.
which enables running benchmarks from the terminal.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The CLI is not only for running benchmarks, it is also for installation, publishing ...

I would keep

Suggested change
which enables simple and easy manipulation of benchmarks just from the terminal.
which enables a simple manipulation of benchmarks from the terminal.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm -1 with removing this part. It is important to hint that the proper way to use benchopt is through the CLI.

As a side note, getting feedback from one user is to be considered not to be directly implemented. It is tricky to please everyone.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, that's why I asked to remove the "running a benchmark through a python file" :) all the doc uses the benchopt cli tool
readding a removed sentence on the benchopt CLI :)

doc/get_started.rst Outdated Show resolved Hide resolved
doc/get_started.rst Outdated Show resolved Hide resolved

benchopt install --env ./benchmark_lasso
Benchopt community maintains :ref:`several optimization benchmarks <available_benchmarks>`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO this information can be ommitted here, documentation should go straight to the point.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it prepares well the floor for using Benchmark Lasso?
Also, It points to the ecosystem of benchopt benchmark, which IMO valuable for the visibility of the benchmark repos.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yesterday's experience with Nelly hinted that there was too much information on the page, which make critical information harder to find. I'll remove this part
The other benchmarks are exposed on the repo's readme

doc/get_started.rst Outdated Show resolved Hide resolved
@mathurinm mathurinm merged commit 4368a19 into benchopt:main Jul 28, 2023
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants