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

metrics namespace per reduction #4498

Open
lalo opened this issue Feb 14, 2023 · 3 comments
Open

metrics namespace per reduction #4498

lalo opened this issue Feb 14, 2023 · 3 comments
Labels
Feature Request New feature requested in system Good First Issue Help wanted PRs welcome

Comments

@lalo
Copy link
Collaborator

lalo commented Feb 14, 2023

Short description

Force reductions to only add metrics in their own namespace. The metrics output dictionary should then be some sort of dictionary[reduction_name_str, dictionary]

Possible solution/implementation details

It can be enforced at learner.h, before calling into the func pointer of persist metrics. You create a new namespace based on the name of the learner and its just a dictionary in json terms (metrics.cc)

Example/links if any

this->learner_ptr->_persist_metrics_f = [fn_ptr, data](metric_sink& metrics) { fn_ptr(*data, metrics); };

@lalo lalo added Feature Request New feature requested in system Help wanted PRs welcome Good First Issue labels Feb 14, 2023
@freyam
Copy link

freyam commented Feb 15, 2023

I want to take this up. I will submit a PR after playing around and testing the patch thoroughly (roughly 1-2 days).

@adityakanu
Copy link

@lalo I am new to the community but this project seems interesting to me. And I believe this could be a good first issue for me to solve. Can you assign this to me?

@Gauravmauryaios
Copy link

Cann you please assign this project .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature Request New feature requested in system Good First Issue Help wanted PRs welcome
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants