-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
refactor: implement scaffold for finish_example split and POC migrations #4296
refactor: implement scaffold for finish_example split and POC migrations #4296
Conversation
|
||
if (_finish_example_fd.print_example_f == nullptr) | ||
THROW("fatal: learner did not register print example fn: " + _name); | ||
if (!has_legacy_print_example()) { THROW("fatal: learner did not register print example fn: " + _name); } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is print_example the legacy print? we should just remove it, its only cb_adf and cb_explore_adf_*
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah it will be superseded by output/record and removed once that is done but until that happens I figured marking it legacy indicates its fate
@@ -55,6 +55,22 @@ void VW::details::return_simple_example(VW::workspace& all, void*, VW::example& | |||
VW::finish_example(all, ec); | |||
} | |||
|
|||
void VW::details::record_stats_simple_label( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe update_stats
instead of record_stats
? it is in sync with sd.update
and it is clear that it has side effects on a shared object since record could just mean record the stats somewhere
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
update_stats
is better I agree
if (ld.label != FLT_MAX && !ec.test_only) { sd.weighted_labels += (static_cast<double>(ld.label)) * ec.weight; } | ||
} | ||
|
||
void VW::details::output_example_simple_label( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this function specific to the label?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this is a general implementation for simple labels
No description provided.