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.

Sign up for GitHub

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

Jump to bottom

refactor: Remove copy label #2921

Merged
merged 6 commits into from Apr 6, 2021
Merged

refactor: Remove copy label #2921

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
dde9f92
Remove copy label
jackgerrits Apr 5, 2021
837150a
formatting
jackgerrits Apr 5, 2021
eccb569
Unused param
jackgerrits Apr 6, 2021
30ea1cf
Fix warnings
jackgerrits Apr 6, 2021
eccd822
Merge branch 'master' into jagerrit/remove_copy_label
jackgerrits Apr 6, 2021
3816113
Merge branch 'master' into jagerrit/remove_copy_label
jackgerrits Apr 6, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 1 addition & 2 deletions test/unit_test/ccb_parser_test.cc
View file
Open in desktop
Expand Up @@ -153,8 +153,7 @@ BOOST_AUTO_TEST_CASE(ccb_copy_label)
auto copied_to = scoped_calloc_or_throw<CCB::label>();
CCB::default_label(*copied_to);

CCB::copy_label(*copied_to, *label);

*copied_to = *label;
BOOST_CHECK_EQUAL(copied_to->explicit_included_actions.size(), 2);
BOOST_CHECK_EQUAL(copied_to->explicit_included_actions[0], 3);
BOOST_CHECK_EQUAL(copied_to->explicit_included_actions[1], 4);
Expand Down
3 changes: 1 addition & 2 deletions test/unit_test/slates_parser_test.cc
View file
Open in desktop
Expand Up @@ -188,8 +188,7 @@ BOOST_AUTO_TEST_CASE(slates_copy_label)

VW::slates::label copied_to;
VW::slates::default_label(copied_to);
VW::slates::copy_label(copied_to, label);

copied_to = label;
BOOST_CHECK_EQUAL(copied_to.type, VW::slates::example_type::slot);
BOOST_CHECK_EQUAL(copied_to.labeled, true);
check_collections_with_float_tolerance(
Expand Down
6 changes: 3 additions & 3 deletions vowpalwabbit/baseline.cc
View file
Open in desktop
Expand Up @@ -107,7 +107,7 @@ void predict_or_learn(baseline& data, single_learner& base, example& ec)
init_global(data);
data.global_initialized = true;
}
VW::copy_example_metadata(/*audit=*/false, data.ec, &ec);
Copy link
Collaborator

Choose a reason for hiding this comment

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

whats the deal with audit being slowly removed

Copy link
Member Author

Choose a reason for hiding this comment

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

Well it wasn't necessary in this function. All audit is copied automatically if it exists. It may be a leftover from how this code used to work

VW::copy_example_metadata(data.ec, &ec);
base.predict(*data.ec);
ec.initial = data.ec->pred.scalar;
base.predict(ec);
Expand All @@ -124,7 +124,7 @@ void predict_or_learn(baseline& data, single_learner& base, example& ec)
if (!data.global_only)
{
// move label & constant features data over to baseline example
VW::copy_example_metadata(/*audit=*/false, data.ec, &ec);
VW::copy_example_metadata(data.ec, &ec);
VW::move_feature_namespace(data.ec, &ec, constant_namespace);
}

Expand Down Expand Up @@ -167,7 +167,7 @@ float sensitivity(baseline& data, base_learner& base, example& ec)
if (!data.global_only) THROW("sensitivity for baseline without --global_only not implemented");

// sensitivity of baseline term
VW::copy_example_metadata(/*audit=*/false, data.ec, &ec);
VW::copy_example_metadata(data.ec, &ec);
data.ec->l.simple.label = ec.l.simple.label;
data.ec->pred.scalar = ec.pred.scalar;
const float baseline_sens = base.sensitivity(*data.ec);
Expand Down
18 changes: 0 additions & 18 deletions vowpalwabbit/cb.cc
View file
Open in desktop
Expand Up @@ -99,12 +99,6 @@ label_parser cb_label = {
[](polylabel* v) { CB::delete_label(v->cb); },
// get_weight
[](polylabel*, const reduction_features&) { return 1.f; },
// copy_label
[](polylabel* dst, polylabel* src) {
if (dst && src) {
CB::copy_label(dst->cb, src->cb);
}
},
// test_label
[](polylabel* v) { return CB::is_test_label(v->cb); },
// post parse processing
Expand Down Expand Up @@ -192,12 +186,6 @@ bool test_label(CB_EVAL::label& ld) { return CB::is_test_label(ld.event); }

void delete_label(CB_EVAL::label& ld) { CB::delete_label(ld.event); }

void copy_label(CB_EVAL::label& dst, CB_EVAL::label& src)
{
CB::copy_label(dst.event, src.event);
dst.action = src.action;
}

void parse_label(parser* p, shared_data* sd, CB_EVAL::label& ld, std::vector<VW::string_view>& words,
reduction_features& red_features)
{
Expand Down Expand Up @@ -228,12 +216,6 @@ label_parser cb_eval = {
[](polylabel* v) { CB_EVAL::delete_label(v->cb_eval); },
// get_weight
[](polylabel*, const reduction_features&) { return 1.f; },
// copy_label
[](polylabel* dst, polylabel* src) {
if (dst && src) {
CB_EVAL::copy_label(dst->cb_eval, src->cb_eval);
}
},
// test_label
[](polylabel* v) { return CB_EVAL::test_label(v->cb_eval); },
// post parse processing
Expand Down
11 changes: 0 additions & 11 deletions vowpalwabbit/cb_continuous_label.cc
View file
Open in desktop
Expand Up @@ -38,11 +38,6 @@ void default_label_additional_fields<VW::cb_continuous::continuous_label>(VW::cb
{
}

template <>
void copy_label_additional_fields<VW::cb_continuous::continuous_label>(
VW::cb_continuous::continuous_label&, VW::cb_continuous::continuous_label&)
{
}
} // namespace CB

void parse_pdf(
Expand Down Expand Up @@ -147,12 +142,6 @@ label_parser the_label_parser = {
// get_weight
// CB::weight just returns 1.f? This seems like it could be a bug...
[](polylabel*, const reduction_features&) { return 1.f; },
// copy_label
[](polylabel* dst, polylabel* src) {
if (dst && src) {
CB::copy_label<continuous_label>(dst->cb_cont, src->cb_cont);
}
},
// test_label
[](polylabel* v) { return CB::is_test_label<continuous_label, continuous_label_elm>(v->cb_cont); },
// post_parse_setup
Expand Down
12 changes: 0 additions & 12 deletions vowpalwabbit/cb_label_parser.h
View file
Open in desktop
Expand Up @@ -124,16 +124,4 @@ void delete_label(LabelT& ld)
ld.costs.delete_v();
}

template <typename LabelT = CB::label>
void copy_label_additional_fields(LabelT& dst, LabelT& src)
{
dst.weight = src.weight;
}

template <typename LabelT = CB::label>
void copy_label(LabelT& dst, LabelT& src)
{
dst.costs = src.costs;
copy_label_additional_fields(dst, src);
}
} // namespace CB
2 changes: 1 addition & 1 deletion vowpalwabbit/cbify.cc
View file
Open in desktop
Expand Up @@ -137,7 +137,7 @@ void cbify_adf_data::copy_example_to_adf(parameters& weights, example& ec)
CB::default_label(lab);

// copy data
VW::copy_example_data(false, &eca, &ec);
VW::copy_example_data(&eca, &ec);

// offset indices for given action
for (features& fs : eca)
Expand Down
22 changes: 0 additions & 22 deletions vowpalwabbit/ccb_label.cc
View file
Open in desktop
Expand Up @@ -184,22 +184,6 @@ void delete_label(label& ld)
ld.explicit_included_actions.delete_v();
}

void copy_label(label& ldDst, label& ldSrc)
{
if (ldSrc.outcome)
{
ldDst.outcome = new CCB::conditional_contextual_bandit_outcome();
ldDst.outcome->probabilities = v_init<ACTION_SCORE::action_score>();

ldDst.outcome->cost = ldSrc.outcome->cost;
ldDst.outcome->probabilities = ldSrc.outcome->probabilities;
}

ldDst.explicit_included_actions = ldSrc.explicit_included_actions;
ldDst.type = ldSrc.type;
ldDst.weight = ldSrc.weight;
}

ACTION_SCORE::action_score convert_to_score(
const VW::string_view& action_id_str, const VW::string_view& probability_str)
{
Expand Down Expand Up @@ -334,12 +318,6 @@ label_parser ccb_label_parser = {
[](polylabel* v) { delete_label(v->conditional_contextual_bandit); },
// get_weight
[](polylabel* v, const ::reduction_features&) { return ccb_weight(v->conditional_contextual_bandit); },
// copy_label
[](polylabel* dst, polylabel* src) {
if (dst && src) {
copy_label(dst->conditional_contextual_bandit, src->conditional_contextual_bandit);
}
},
// test_label
[](polylabel* v) { return test_label(v->conditional_contextual_bandit); },
// parse post processing
Expand Down
1 change: 0 additions & 1 deletion vowpalwabbit/ccb_label.h
View file
Open in desktop
Expand Up @@ -108,7 +108,6 @@ void delete_label(CCB::label& ld);
void parse_label(parser* p, shared_data*, CCB::label& ld, std::vector<VW::string_view>& words, ::reduction_features&);
void cache_label(CCB::label& ld, io_buf& cache);
size_t read_cached_label(shared_data*, CCB::label& ld, io_buf& cache);
void copy_label(CCB::label& ldDst, CCB::label& ldSrc);

extern label_parser ccb_label_parser;
} // namespace CCB
8 changes: 0 additions & 8 deletions vowpalwabbit/cost_sensitive.cc
View file
Open in desktop
Expand Up @@ -101,8 +101,6 @@ bool test_label(label& ld)

void delete_label(label& ld) { ld.costs.delete_v(); }

void copy_label(label& dst, label& src) { dst.costs = src.costs; }

void parse_label(parser* p, shared_data* sd, label& ld, std::vector<VW::string_view>& words, reduction_features&)
{
ld.costs.clear();
Expand Down Expand Up @@ -183,12 +181,6 @@ label_parser cs_label = {
[](polylabel* v) { if (v) delete_label(v->cs); },
// get_weight
[](polylabel* v, const reduction_features&) { return weight(v->cs); },
// copy_label
[](polylabel* dst, polylabel* src) {
if (dst && src) {
copy_label(dst->cs, src->cs);
}
},
// test_label
[](polylabel* v) { return test_label(v->cs); },
// parse post processing
Expand Down
22 changes: 17 additions & 5 deletions vowpalwabbit/example.cc
View file
Open in desktop
Expand Up @@ -74,7 +74,9 @@ void copy_example_label(example* dst, example* src, void (*copy_label)(polylabel
dst->l = src->l;
}

void copy_example_metadata(bool /* audit */, example* dst, example* src)
void copy_example_label(example* dst, const example* src) { dst->l = src->l; }

void copy_example_metadata(example* dst, const example* src)
{
dst->tag = src->tag;
dst->example_counter = src->example_counter;
Expand All @@ -99,9 +101,9 @@ void copy_example_metadata(bool /* audit */, example* dst, example* src)
dst->initial = src->initial;
}

void copy_example_data(bool audit, example* dst, example* src)
void copy_example_data(example* dst, const example* src)
{
copy_example_metadata(audit, dst, src);
copy_example_metadata(dst, src);

// copy feature data
dst->indices = src->indices;
Expand All @@ -112,9 +114,13 @@ void copy_example_data(bool audit, example* dst, example* src)
dst->_debug_current_reduction_depth = src->_debug_current_reduction_depth;
}

void copy_example_data(bool audit, example* dst, example* src, void (*copy_label)(polylabel*, polylabel*))
void copy_example_metadata(bool /* audit */, example* dst, example* src) { copy_example_metadata(dst, src); }

void copy_example_data(bool /* audit */, example* dst, example* src) { copy_example_data(dst, src); }

void copy_example_data(bool /* audit */, example* dst, example* src, void (*copy_label)(polylabel*, polylabel*))
{
copy_example_data(audit, dst, src);
copy_example_data(dst, src);
copy_example_label(dst, src, copy_label);
}

Expand All @@ -124,6 +130,12 @@ void copy_example_data(
copy_example_data(audit, dst, src, copy_label);
}

void copy_example_data_with_label(example* dst, const example* src)
{
copy_example_data(dst, src);
copy_example_label(dst, src);
}

void move_feature_namespace(example* dst, example* src, namespace_index c)
{
if (std::find(src->indices.begin(), src->indices.end(), c) == src->indices.end()) return; // index not present in src
Expand Down
6 changes: 1 addition & 5 deletions vowpalwabbit/expreplay.h
View file
Open in desktop
Expand Up @@ -47,11 +47,7 @@ void learn(expreplay<lp> &er, LEARNER::single_learner &base, example &ec)
if (er.filled[n]) base.learn(er.buf[n]);

er.filled[n] = true;
VW::copy_example_data(er.all->audit, &er.buf[n], &ec); // don't copy the label
if (lp.copy_label)
lp.copy_label(&er.buf[n].l, &ec.l);
else
er.buf[n].l = ec.l;
VW::copy_example_data_with_label(&er.buf[n], &ec);
}

template <label_parser &lp>
Expand Down
2 changes: 1 addition & 1 deletion vowpalwabbit/ezexample.h
View file
Open in desktop
Expand Up @@ -273,7 +273,7 @@ class ezexample
else // is multiline
{ // we need to make a copy
example* copy = get_new_example();
VW::copy_example_data(vw_ref->audit, copy, ec, vw_par_ref->example_parser->lbl_parser.copy_label);
VW::copy_example_data_with_label(copy, ec);
vw_ref->learn(*copy);
example_copies.push_back(copy);
}
Expand Down
5 changes: 0 additions & 5 deletions vowpalwabbit/label_parser.h
View file
Open in desktop
Expand Up @@ -38,11 +38,6 @@ struct label_parser
size_t (*read_cached_label)(shared_data*, polylabel*, reduction_features&, io_buf& cache);
void (*delete_label)(polylabel*);
float (*get_weight)(polylabel*, const reduction_features&);
void (*copy_label)(
polylabel*, polylabel*); // copy_label(dst,src) performs a DEEP copy of src into dst (dst is allocated
// correctly). if this function is nullptr, then we assume that a memcpy of size
// label_size is sufficient, so you need only specify this function if your label
// constains, for instance, pointers (otherwise you'll get double-free errors)
bool (*test_label)(polylabel*);
void (*post_parse_setup)(example*);
label_type_t label_type;
Expand Down
2 changes: 1 addition & 1 deletion vowpalwabbit/memory_tree.cc
View file
Open in desktop
Expand Up @@ -52,7 +52,7 @@ void copy_example_data(example* dst, example* src, bool oas = false) // copy ex
{
dst->l.multilabels.label_v = src->l.multilabels.label_v;
}
VW::copy_example_data(false, dst, src);
VW::copy_example_data(dst, src);
}

////Implement kronecker_product between two examples:
Expand Down
2 changes: 0 additions & 2 deletions vowpalwabbit/multiclass.cc
View file
Open in desktop
Expand Up @@ -101,8 +101,6 @@ label_parser mc_label = {
[](polylabel*) {},
// get_weight
[](polylabel* v, const reduction_features&) { return weight(v->multi); },
// copy_label
nullptr,
// test_label
[](polylabel* v) { return test_label(v->multi); },
// post parse processing
Expand Down
8 changes: 0 additions & 8 deletions vowpalwabbit/multilabel.cc
View file
Open in desktop
Expand Up @@ -76,8 +76,6 @@ bool test_label(MULTILABEL::labels& ld) { return ld.label_v.size() == 0; }

void delete_label(MULTILABEL::labels& ld) { ld.label_v.delete_v(); }

void copy_label(MULTILABEL::labels& dst, MULTILABEL::labels& src) { dst.label_v = src.label_v; }

void parse_label(
parser* p, shared_data*, MULTILABEL::labels& ld, std::vector<VW::string_view>& words, reduction_features&)
{
Expand Down Expand Up @@ -115,12 +113,6 @@ label_parser multilabel = {
[](polylabel* v) { if (v) delete_label(v->multilabels); },
// get_weight
[](polylabel* v, const reduction_features&) { return weight(v->multilabels); },
// copy_label
[](polylabel* dst, polylabel* src) {
if (dst && src) {
copy_label(dst->multilabels, src->multilabels);
}
},
// test_label
[](polylabel* v) { return test_label(v->multilabels); },
// post parse processing
Expand Down
2 changes: 0 additions & 2 deletions vowpalwabbit/no_label.cc
View file
Open in desktop
Expand Up @@ -50,8 +50,6 @@ label_parser no_label_parser = {
[](polylabel*) {},
// get_weight
[](polylabel*, const reduction_features&) { return 1.f; },
// copy_label
nullptr,
// test_label
[](polylabel*) { return false; },
// post parse processing
Expand Down
7 changes: 2 additions & 5 deletions vowpalwabbit/search.cc
View file
Open in desktop
Expand Up @@ -1648,11 +1648,8 @@ action search_predict(search_private& priv, example* ecs, size_t ec_cnt, ptag my
priv.learn_ec_ref = ecs;
else
{
void (*label_copy_fn)(polylabel*, polylabel*) = priv.is_ldf ? CS::cs_label.copy_label : nullptr;

priv.learn_ec_copy.resize(ec_cnt);
for (size_t i = 0; i < ec_cnt; i++)
VW::copy_example_data(priv.all->audit, &priv.learn_ec_copy[i], ecs + i, label_copy_fn);
for (size_t i = 0; i < ec_cnt; i++) { VW::copy_example_data_with_label(&priv.learn_ec_copy[i], ecs + i); }

priv.learn_ec_ref = priv.learn_ec_copy.data();
}
Expand Down Expand Up @@ -3072,7 +3069,7 @@ void predictor::set_input_at(size_t posn, example& ex)
if (posn >= ec_cnt)
THROW("call to set_input_at with too large a position: posn (" << posn << ") >= ec_cnt(" << ec_cnt << ")");

VW::copy_example_data(false, ec + posn, &ex, CS::cs_label.copy_label); // TODO: the false is "audit"
VW::copy_example_data_with_label(ec + posn, &ex);
}

predictor& predictor::erase_oracles()
Expand Down
4 changes: 2 additions & 2 deletions vowpalwabbit/search_entityrelationtask.cc
View file
Open in desktop
Expand Up @@ -158,7 +158,7 @@ size_t predict_entity(
{
for (uint32_t a = 0; a < 4; a++)
{
VW::copy_example_data(false, &my_task_data->ldf_entity[a], ex);
VW::copy_example_data(&my_task_data->ldf_entity[a], ex);
update_example_indicies(true, &my_task_data->ldf_entity[a], 28904713, 4832917 * (uint64_t)(a + 1));
CS::label& lab = my_task_data->ldf_entity[a].l.cs;
lab.costs[0].x = 0.f;
Expand Down Expand Up @@ -241,7 +241,7 @@ size_t predict_relation(Search::search& sch, example* ex, v_array<size_t>& predi
int correct_label = 0; // if correct label is not in the set, use the first one
for (size_t a = 0; a < constrained_relation_labels.size(); a++)
{
VW::copy_example_data(false, &my_task_data->ldf_relation[a], ex);
VW::copy_example_data(&my_task_data->ldf_relation[a], ex);
update_example_indicies(
true, &my_task_data->ldf_relation[a], 28904713, 4832917 * (uint64_t)(constrained_relation_labels[a]));
CS::label& lab = my_task_data->ldf_relation[a].l.cs;
Expand Down
2 changes: 1 addition & 1 deletion vowpalwabbit/search_sequencetask.cc
View file
Open in desktop
Expand Up @@ -418,7 +418,7 @@ void run(Search::search& sch, multi_ex& ec)
{
if (sch.predictNeedsExample()) // we can skip this work if `predict` won't actually use the example data
{
VW::copy_example_data(false, &data->ldf_examples[a], ec[i]); // copy but leave label alone!
VW::copy_example_data(&data->ldf_examples[a], ec[i]); // copy but leave label alone!
// now, offset it appropriately for the action id
my_update_example_indicies(sch, true, &data->ldf_examples[a], 28904713, 4832917 * (uint64_t)a);
}
Expand Down
2 changes: 0 additions & 2 deletions vowpalwabbit/simple_label.cc
View file
Open in desktop
Expand Up @@ -128,8 +128,6 @@ label_parser simple_label_parser = {
[](polylabel*) {},
// get_weight
[](polylabel* v, const reduction_features&) { return get_weight(v->simple); },
// copy_label
nullptr,
// test_label
[](polylabel* v) { return test_label(v->simple); },
// test_label
Expand Down