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

Implement a tag field for BaseSolver #586

Draft
wants to merge 14 commits into
base: main
Choose a base branch
from

Conversation

svaiter
Copy link

@svaiter svaiter commented Jul 4, 2023

  • Implement a tag field for BaseSolver (e.g. tags = ['proximal_coordinate_descent', 'python'])
  • Serialize it in the html output
  • Write the necessary JS logic to filter according to this list of tag
  • Allow a CLI argument to only run solvers satisfying a given tag

(issue #571)

@Badr-MOUFAD Badr-MOUFAD linked an issue Jul 10, 2023 that may be closed by this pull request
@Badr-MOUFAD
Copy link
Member

Here is the preview of the visualization feature, and the benchmark repository to reproduce it.
Feedback is very welcome, @svaiter.

Also, how should we handle visualization conflicts between tags and solvers?
In fact, tags are ineffective for solvers hidden by clicking on the legend. I'm thinking about re-initializing the filtering whenever the tags are used. any thoughts?

@svaiter
Copy link
Author

svaiter commented Jul 10, 2023

Excellent ! Thank you.

In my opinion,

  • Clicking a dedicated solver should deactivate the tags.
  • Reciprocally, clicking a tag should reinitialise the state of the hidden solvers.
  • I would prefer I think if the tag cloud is either at the end of the column, or below the solver list.
  • I don't know how we should handle if there is a lot of different tag. Maybe a search area allowing to select the tag ? (maybe this is premature optimization...)

benchopt/runner.py Outdated Show resolved Hide resolved
Co-authored-by: Thomas Moreau <thomas.moreau.2010@gmail.com>
Copy link

codecov bot commented May 31, 2024

Codecov Report

Attention: Patch coverage is 5.55556% with 17 lines in your changes are missing coverage. Please review.

Project coverage is 54.60%. Comparing base (18fa53d) to head (2f38264).

Current head 2f38264 differs from pull request most recent head 62726cf

Please upload reports for the commit 62726cf to get more accurate results.

Additional details and impacted files
@@             Coverage Diff             @@
##             main     #586       +/-   ##
===========================================
- Coverage   80.80%   54.60%   -26.20%     
===========================================
  Files          45       44        -1     
  Lines        3027     2864      -163     
  Branches        0      529      +529     
===========================================
- Hits         2446     1564      -882     
- Misses        581     1181      +600     
- Partials        0      119      +119     

Copy link
Member

@tomMoral tomMoral left a comment

Choose a reason for hiding this comment

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

This is a nice start, would be interesting to finish this one during the sprint?

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.

Add a "tag" member to solvers (allow filtering solvers)
3 participants